[PATCH 1/3] Adding Skyworks SKY81452 MFD driver

Post by Gyungoh Yoo
Can somebody please review if the code has anything to improve?

- Please don't top post - if you have a comment about the code, make
your comment below the line(s) you are referring to and cut out all
surplus quotes, replacing with "[...]".

- This driver was sent on Friday 4 days (1.5 working days) ago -
please leave at least 2 weeks before 're-sending' your patches - don't
poke.

- The merge window is currently open - maintainers are usually busy
sending pull-requests to Linus or taking a few days to have a breather
after the previous cycle - things will ramp up again in a week or so.

Someone will get round to reviewing your driver in due time. Please
be patient.

Post by Gyungoh Yoo
=20

txt

create mode 100644 drivers/mfd/sky81452.c
create mode 100644 include/linux/mfd/sky81452.h
diff --git a/Documentation/devicetree/bindings/mfd/sky81452.txt
b/Documentation/devicetree/bindings/mfd/sky81452.txt
new file mode 100644
index 0000000..5fb0b4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/sky81452.txt
@@ -0,0 +1,24 @@
+SKY81452 bindings
+
+- compatible : Must be "skyworks,sky81452"
+
+- backlight : container node for backlight following the bindin=

+ in video/backlight/sky81452-backlight.txt
+- regulator : container node for regulators following the bindi=

+ in regulator/sky81452-regulator.txt
+
+
+ compatible =3D "skyworks,sky81452";
+
+ backlight {
+ ...
+ };
+
+ regulator {
+ ...
+ };
+ };
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt
b/Documentation/devicetree/bindings/vendor-prefixes.txt
index d415b38..ce76e10 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -122,6 +122,7 @@ silabs Silicon Laboratories
simtek
sii Seiko Instruments, Inc.
sirf SiRF Technology, Inc.
+skyworks Skyworks Solutions, Inc.
smsc Standard Microsystems Corporation
snps Synopsys, Inc.
spansion Spansion Inc.
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de5abf2..acfb2e5 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -626,6 +626,18 @@ config MFD_SM501_GPIO
lines on the SM501. The platform data is used to supply th=

base number for the first GPIO line to register.
+config SKY81452
+ tristate "Skyworks Solutions SKY81452"
+ select MFD_CORE
+ select REGMAP_I2C
+ depends on I2C=3Dy
+ help
+ This is the core driver for the Skyworks SKY81452 backlig=

ht and

+ voltage regulator device.
+
+ This driver can also be built as a module. If so, the mo=

dule

+ will be called sky81452.
+
config MFD_SMSC
bool "SMSC ECE1099 series chips"
depends on I2C=3Dy
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f001487..191c656 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -169,6 +169,7 @@ obj-$(CONFIG_MFD_AS3711) +=3D as3711.o
obj-$(CONFIG_MFD_AS3722) +=3D as3722.o
obj-$(CONFIG_MFD_STW481X) +=3D stw481x.o
obj-$(CONFIG_MFD_IPAQ_MICRO) +=3D ipaq-micro.o
+obj-$(CONFIG_SKY81452) +=3D sky81452.o
intel-soc-pmic-objs :=3D intel_soc_pmic_core.o
intel_soc_pmic_crc.o
obj-$(CONFIG_INTEL_SOC_PMIC) +=3D intel-soc-pmic.o
diff --git a/drivers/mfd/sky81452.c b/drivers/mfd/sky81452.c
new file mode 100644
index 0000000..566912f
--- /dev/null
+++ b/drivers/mfd/sky81452.c
@@ -0,0 +1,113 @@
+/*
+ * sky81452.c SKY81452 MFD driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ *
+ * This program is free software; you can redistribute it and/or m=

odify it

+ * under the terms of the GNU General Public License as published =

by the

+ * Free Software Foundation; either version 2, or (at your option)=

any

+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful,=

but

+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the G=

+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public Licen=

se along

+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/sky81452.h>
+
+static const struct regmap_config sky81452_config =3D {
+ .reg_bits =3D 8,
+ .val_bits =3D 8,
+};
+
+static int sky81452_register_devices(struct device *dev,
+ const struct sky81452_platform_data *pdata)
+{
+ struct mfd_cell cells[] =3D {
+ {
+ .name =3D "sky81452-bl",
+ .platform_data =3D pdata->bl_pdata,
+ .pdata_size =3D sizeof(*pdata->bl_pdata),
+ },
+ {
+ .name =3D "sky81452-regulator",
+ .platform_data =3D pdata->regulator_init_da=

ta,

+ .pdata_size =3D sizeof(*pdata->regulator_in=

it_data),

+ },
+ };
+
+ return mfd_add_devices(dev, -1, cells, ARRAY_SIZE(cells),
+ NULL, 0, NULL);
+}
+
+static int sky81452_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device *dev =3D &client->dev;
+ const struct sky81452_platform_data *pdata =3D dev_get_plat=

data(dev);

+ struct regmap *map;
+
+ if (!pdata) {
+ pdata =3D devm_kzalloc(dev, sizeof(*pdata), GFP_KER=

NEL);

+ if (!pdata)
+ return -ENOMEM;
+ }
+
+ map =3D devm_regmap_init_i2c(client, &sky81452_config);
+ if (IS_ERR(map))
+ return PTR_ERR(map);
+
+ i2c_set_clientdata(client, map);
+
+ return sky81452_register_devices(dev, pdata);
+}
+
+static int sky81452_remove(struct i2c_client *client)
+{
+ mfd_remove_devices(&client->dev);
+ return 0;
+}
+
+static const struct i2c_device_id sky81452_ids[] =3D {
+ { "sky81452", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, sky81452_ids);
+
+#ifdef CONFIG_OF
+static const struct of_device_id sky81452_of_match[] =3D {
+ { .compatible =3D "skyworks,sky81452", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sky81452_of_match);
+#endif
+
+static struct i2c_driver sky81452_driver =3D {
+ .driver =3D {
+ .name =3D "sky81452",
+ .of_match_table =3D of_match_ptr(sky81452_of_match)=

+ },
+ .probe =3D sky81452_probe,
+ .remove =3D sky81452_remove,
+ .id_table =3D sky81452_ids,
+};
+
+module_i2c_driver(sky81452_driver);
+
+MODULE_DESCRIPTION("Skyworks SKY81452 MFD driver");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("1.0");
diff --git a/include/linux/mfd/sky81452.h b/include/linux/mfd/sky81=

452.h

new file mode 100644
index 0000000..8d8ed35
--- /dev/null
+++ b/include/linux/mfd/sky81452.h
@@ -0,0 +1,32 @@
+/*
+ * sky81452.h SKY81452 backlight driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ *
+ * This program is free software; you can redistribute it and/or m=

odify it

+ * under the terms of the GNU General Public License as published =

by the

+ * Free Software Foundation; either version 2, or (at your option)=

any

+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful,=

but

+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the G=

+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public Licen=

se along

+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _SKY81452_H
+#define _SKY81452_H
+
+#include "linux/sky81452-backlight.h"
+#include "linux/regulator/machine.h"
+
+struct sky81452_platform_data {
+ struct sky81452_bl_platform_data *bl_pdata;
+ struct regulator_init_data *regulator_init_data;
+};
+
+#endif

--=20
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org =E2=94=82 Open source software for ARM SoCs
=46ollow Linaro: Facebook | Twitter | Blog

Lee Jones

2014-08-21 09:45:02 UTC

When you send patch-sets, you should send them connected to one
another AKA threaded. That way, when we're reviewing we can look at
the other patches in the set for reference. See the man page for `git
send-email` for details.

<commit log>

---
Documentation/devicetree/bindings/mfd/sky81452.txt | 24 +++++

Binding documents should be sent separately:

See: Documentation/devicetree/bindings/submitting-patches.txt

.../devicetree/bindings/vendor-prefixes.txt | 1 +

This can go in with the documentation.

drivers/mfd/Kconfig | 12 +++
drivers/mfd/Makefile | 1 +
drivers/mfd/sky81452.c | 113 +++++++++++=

++++++++++

include/linux/mfd/sky81452.h | 32 ++++++
6 files changed, 183 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/sky81452.tx=

create mode 100644 drivers/mfd/sky81452.c
create mode 100644 include/linux/mfd/sky81452.h
=20
diff --git a/Documentation/devicetree/bindings/mfd/sky81452.txt b/Doc=

umentation/devicetree/bindings/mfd/sky81452.txt

new file mode 100644
index 0000000..5fb0b4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/sky81452.txt
@@ -0,0 +1,24 @@
+SKY81452 bindings
+
+- compatible : Must be "skyworks,sky81452"
+
+- backlight : container node for backlight following the binding
+ in video/backlight/sky81452-backlight.txt
+- regulator : container node for regulators following the binding
+ in regulator/sky81452-regulator.txt
+
+

Lower case.

+ compatible =3D "skyworks,sky81452";

"reg"?

You also need to document the 'compatible' and 'reg' properties.

+ backlight {
+ ...
+ };
+
+ regulator {
+ ...
+ };
+ };

I think it would be helpful to place a fully populated example in
here.

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/=

Documentation/devicetree/bindings/vendor-prefixes.txt

index d415b38..ce76e10 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -122,6 +122,7 @@ silabs Silicon Laboratories
simtek
sii Seiko Instruments, Inc.
sirf SiRF Technology, Inc.
+skyworks Skyworks Solutions, Inc.
smsc Standard Microsystems Corporation
snps Synopsys, Inc.
spansion Spansion Inc.
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de5abf2..acfb2e5 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -626,6 +626,18 @@ config MFD_SM501_GPIO
lines on the SM501. The platform data is used to supply the
base number for the first GPIO line to register.
=20
+config SKY81452

MFD_SKY81452

+ tristate "Skyworks Solutions SKY81452"
+ select MFD_CORE
+ select REGMAP_I2C
+ depends on I2C=3Dy

Why do you need I2C to be built-in, rather than as a module?

+ help
+ This is the core driver for the Skyworks SKY81452 backlight and
+ voltage regulator device.
+
+ This driver can also be built as a module. If so, the module
+ will be called sky81452.
+
config MFD_SMSC
bool "SMSC ECE1099 series chips"
depends on I2C=3Dy
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f001487..191c656 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -169,6 +169,7 @@ obj-$(CONFIG_MFD_AS3711) +=3D as3711.o
obj-$(CONFIG_MFD_AS3722) +=3D as3722.o
obj-$(CONFIG_MFD_STW481X) +=3D stw481x.o
obj-$(CONFIG_MFD_IPAQ_MICRO) +=3D ipaq-micro.o
+obj-$(CONFIG_SKY81452) +=3D sky81452.o
=20
intel-soc-pmic-objs :=3D intel_soc_pmic_core.o intel_soc_pmic_crc.o
obj-$(CONFIG_INTEL_SOC_PMIC) +=3D intel-soc-pmic.o
diff --git a/drivers/mfd/sky81452.c b/drivers/mfd/sky81452.c
new file mode 100644
index 0000000..566912f
--- /dev/null
+++ b/drivers/mfd/sky81452.c
@@ -0,0 +1,113 @@
+/*
+ * sky81452.c SKY81452 MFD driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ *
+ * This program is free software; you can redistribute it and/or mod=

ify it

+ * under the terms of the GNU General Public License as published by=

the

+ * Free Software Foundation; either version 2, or (at your option) a=

+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, b=

+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License=

along

Have you tested this with DT?

You're not passing the compatible string and not using
of_platform_populate() so I'm struggling to see how it would work
properly.

+ },
+ {
+ .name =3D "sky81452-regulator",
+ .platform_data =3D pdata->regulator_init_data,
+ .pdata_size =3D sizeof(*pdata->regulator_init_data),
+ },
+ };

Please declare this outside of the function?

+ return mfd_add_devices(dev, -1, cells, ARRAY_SIZE(cells),
+ NULL, 0, NULL);

This doesn't really need to be in a function of its own. Please put
it in .probe(). Also check for the return value and present the user
with an error message if it fails.

+}
+
+static int sky81452_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)

Line up against '('.

+{
+ struct device *dev =3D &client->dev;
+ const struct sky81452_platform_data *pdata =3D dev_get_platdata(dev=

);

+ struct regmap *map;

Can you call this 'regmap' for clarity.

+ if (!pdata) {
+ pdata =3D devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+ }
+
+ map =3D devm_regmap_init_i2c(client, &sky81452_config);
+ if (IS_ERR(map))
+ return PTR_ERR(map);

I'm not sure you want this to fail silently.

+ i2c_set_clientdata(client, map);
+
+ return sky81452_register_devices(dev, pdata);
+}
+
+static int sky81452_remove(struct i2c_client *client)
+{
+ mfd_remove_devices(&client->dev);
+ return 0;
+}
+
+static const struct i2c_device_id sky81452_ids[] =3D {
+ { "sky81452", 0 },

Remove the second attribute, it's unused.

+ { }
+};
+MODULE_DEVICE_TABLE(i2c, sky81452_ids);
+
+#ifdef CONFIG_OF
+static const struct of_device_id sky81452_of_match[] =3D {
+ { .compatible =3D "skyworks,sky81452", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sky81452_of_match);
+#endif

You can drop the #differy the compiler should sort that out on the
back of of_match_ptr().

+static struct i2c_driver sky81452_driver =3D {
+ .driver =3D {
+ .name =3D "sky81452",
+ .of_match_table =3D of_match_ptr(sky81452_of_match),
+ },
+ .probe =3D sky81452_probe,
+ .remove =3D sky81452_remove,
+ .id_table =3D sky81452_ids,
+};
+
+module_i2c_driver(sky81452_driver);
+
+MODULE_DESCRIPTION("Skyworks SKY81452 MFD driver");
+MODULE_LICENSE("GPL");

I think you want v2.

+MODULE_VERSION("1.0");
diff --git a/include/linux/mfd/sky81452.h b/include/linux/mfd/sky8145=

2.h

new file mode 100644
index 0000000..8d8ed35
--- /dev/null
+++ b/include/linux/mfd/sky81452.h
@@ -0,0 +1,32 @@
+/*
+ * sky81452.h SKY81452 backlight driver

If it's a just a backlight driver, why is it in MFD?

You'd best expand the description here.

+ * Copyright 2014 Skyworks Solutions Inc.
+ *
+ * This program is free software; you can redistribute it and/or mod=

ify it

+ * under the terms of the GNU General Public License as published by=

the

+ * Free Software Foundation; either version 2, or (at your option) a=

+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, b=

along

+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _SKY81452_H
+#define _SKY81452_H
+
+#include "linux/sky81452-backlight.h"
+#include "linux/regulator/machine.h"

s/"/< and >

+struct sky81452_platform_data {
+ struct sky81452_bl_platform_data *bl_pdata;
+ struct regulator_init_data *regulator_init_data;
+};
+
+#endif

--=20
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org =E2=94=82 Open source software for ARM SoCs
=46ollow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" i=
n
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Thierry Reding

2014-08-21 09:59:36 UTC

On Thu, Aug 21, 2014 at 10:45:02AM +0100, Lee Jones wrote:
[...]

diff --git a/drivers/mfd/sky81452.c b/drivers/mfd/sky81452.c

[...]

+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.

[...]

+ { }
+};
+MODULE_DEVICE_TABLE(i2c, sky81452_ids);
+
+#ifdef CONFIG_OF
+static const struct of_device_id sky81452_of_match[] = {
+ { .compatible = "skyworks,sky81452", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sky81452_of_match);
+#endif

You can drop the #differy the compiler should sort that out on the
back of of_match_ptr().

It won't, unfortunately. If !OF, then of_match_ptr(x) will evaluate to
NULL, therefore sky81452_of_match will become unreferenced and cause the
compiler to output a warning. I guess it could be made __maybe_unused,
but I don't see that as much of an advantage over the #ifdef.

+static struct i2c_driver sky81452_driver = {
+ .driver = {
+ .name = "sky81452",
+ .of_match_table = of_match_ptr(sky81452_of_match),
+ },
+ .probe = sky81452_probe,
+ .remove = sky81452_remove,
+ .id_table = sky81452_ids,
+};
+
+module_i2c_driver(sky81452_driver);
+
+MODULE_DESCRIPTION("Skyworks SKY81452 MFD driver");
+MODULE_LICENSE("GPL");

I think you want v2.

The header comment says "... version 2, or (at your option) any later
version.", so I think "GPL" is the right one here.

Thierry

Lee Jones

2014-08-21 11:19:40 UTC

[...]

diff --git a/drivers/mfd/sky81452.c b/drivers/mfd/sky81452.c

[...]

+ * This program is free software; you can redistribute it and/or=

modify it

+ * under the terms of the GNU General Public License as publishe=

d by the

+ * Free Software Foundation; either version 2, or (at your optio=

n) any

+ * later version.

[...]

=20
You can drop the #differy the compiler should sort that out on the
back of of_match_ptr().

=20
It won't, unfortunately. If !OF, then of_match_ptr(x) will evaluate t=

NULL, therefore sky81452_of_match will become unreferenced and cause =

the

compiler to output a warning. I guess it could be made __maybe_unused=

but I don't see that as much of an advantage over the #ifdef.

Yes, I suppose it would. That's sad.

=20
I think you want v2.

=20
The header comment says "... version 2, or (at your option) any later
version.", so I think "GPL" is the right one here.

=46air enough. Withdrawn.

--=20
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org =E2=94=82 Open source software for ARM SoCs
=46ollow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" i=
n
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Gyungoh Yoo

2014-08-25 07:06:08 UTC

Thank you for reviewing the patches.

Post by Lee Jones
When you send patch-sets, you should send them connected to one
another AKA threaded. That way, when we're reviewing we can look at
the other patches in the set for reference. See the man page for `gi=

Post by Lee Jones
send-email` for details.
=20
<commit log>
=20

---
Documentation/devicetree/bindings/mfd/sky81452.txt | 24 +++++

=20
=20
See: Documentation/devicetree/bindings/submitting-patches.txt
=20

.../devicetree/bindings/vendor-prefixes.txt | 1 +

=20
This can go in with the documentation.

I see. I will re-send them separately.

drivers/mfd/Kconfig | 12 +++
drivers/mfd/Makefile | 1 +
drivers/mfd/sky81452.c | 113 +++++++++=

++++++++++++

include/linux/mfd/sky81452.h | 32 ++++++
6 files changed, 183 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/sky81452.=

txt

create mode 100644 drivers/mfd/sky81452.c
create mode 100644 include/linux/mfd/sky81452.h
=20
diff --git a/Documentation/devicetree/bindings/mfd/sky81452.txt b/D=

ocumentation/devicetree/bindings/mfd/sky81452.txt

=20
Lower case.
=20

+ compatible =3D "skyworks,sky81452";

=20
"reg"?
=20
You also need to document the 'compatible' and 'reg' properties.
=20

+ backlight {
+ ...
+ };
+
+ regulator {
+ ...
+ };
+ };

=20
I think it would be helpful to place a fully populated example in
here.

I will update it.

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt =

b/Documentation/devicetree/bindings/vendor-prefixes.txt

=20
MFD_SKY81452
=20

+ tristate "Skyworks Solutions SKY81452"
+ select MFD_CORE
+ select REGMAP_I2C
+ depends on I2C=3Dy

=20
Why do you need I2C to be built-in, rather than as a module?

It will depend only on I2C.

=2Eo

obj-$(CONFIG_INTEL_SOC_PMIC) +=3D intel-soc-pmic.o
diff --git a/drivers/mfd/sky81452.c b/drivers/mfd/sky81452.c
new file mode 100644
index 0000000..566912f
--- /dev/null
+++ b/drivers/mfd/sky81452.c
@@ -0,0 +1,113 @@
+/*
+ * sky81452.c SKY81452 MFD driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ *
+ * This program is free software; you can redistribute it and/or m=

odify it

+ * under the terms of the GNU General Public License as published =

by the

+ * Free Software Foundation; either version 2, or (at your option)=

any

+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful,=

but

+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the G=

+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public Licen=

se along

=20
Have you tested this with DT?
=20
You're not passing the compatible string and not using
of_platform_populate() so I'm struggling to see how it would work
properly.

sky81452-bl and regulator-sky81452 is parsing the information
in regulator node of its parent node. So I thought these 2 drivers
don't need compatible attribute. That is what it didn't have
compatible string.
Is is mandatory that all drivers should have compatible attribute?

+ },
+ {
+ .name =3D "sky81452-regulator",
+ .platform_data =3D pdata->regulator_init_data,
+ .pdata_size =3D sizeof(*pdata->regulator_init_data),
+ },
+ };

=20
Please declare this outside of the function?

I see.

+ return mfd_add_devices(dev, -1, cells, ARRAY_SIZE(cells),
+ NULL, 0, NULL);

=20
This doesn't really need to be in a function of its own. Please put
it in .probe(). Also check for the return value and present the user
with an error message if it fails.

I think this need to be, in case of !CONFIG_OF.
Can you please explain more in details?

+}
+
+static int sky81452_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)

=20
Line up against '('.
=20

+{
+ struct device *dev =3D &client->dev;
+ const struct sky81452_platform_data *pdata =3D dev_get_platdata(d=

ev);

+ struct regmap *map;

=20
Can you call this 'regmap' for clarity.

I see. Thank you.

=20
I'm not sure you want this to fail silently.
=20

=20
Remove the second attribute, it's unused.
=20

=20
You can drop the #differy the compiler should sort that out on the
back of of_match_ptr().
=20

=20
I think you want v2.
=20

+MODULE_VERSION("1.0");
diff --git a/include/linux/mfd/sky81452.h b/include/linux/mfd/sky81=

452.h

new file mode 100644
index 0000000..8d8ed35
--- /dev/null
+++ b/include/linux/mfd/sky81452.h
@@ -0,0 +1,32 @@
+/*
+ * sky81452.h SKY81452 backlight driver

=20
If it's a just a backlight driver, why is it in MFD?
=20
You'd best expand the description here.

Sorry about that. I missed it.

+ * Copyright 2014 Skyworks Solutions Inc.
+ *
+ * This program is free software; you can redistribute it and/or m=

odify it

+ * under the terms of the GNU General Public License as published =

by the

+ * Free Software Foundation; either version 2, or (at your option)=

any

+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful,=

but

+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the G=

+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public Licen=

se along

+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _SKY81452_H
+#define _SKY81452_H
+
+#include "linux/sky81452-backlight.h"
+#include "linux/regulator/machine.h"

=20
s/"/< and >
=20

+struct sky81452_platform_data {
+ struct sky81452_bl_platform_data *bl_pdata;
+ struct regulator_init_data *regulator_init_data;
+};
+
+#endif

=20
--=20
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org =E2=94=82 Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

--
To unsubscribe from this list: send the line "unsubscribe devicetree" i=
n
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Lee Jones

2014-08-26 08:22:58 UTC

Post by Lee Jones
When you send patch-sets, you should send them connected to one
another AKA threaded. That way, when we're reviewing we can look a=

Post by Lee Jones
the other patches in the set for reference. See the man page for `=

git

Post by Lee Jones
send-email` for details.
=20
<commit log>
=20

---

[...]

+static int sky81452_register_devices(struct device *dev,
+ const struct sky81452_platform_data *pdata)
+{
+ struct mfd_cell cells[] =3D {
+ {
+ .name =3D "sky81452-bl",
+ .platform_data =3D pdata->bl_pdata,
+ .pdata_size =3D sizeof(*pdata->bl_pdata),

=20
Have you tested this with DT?
=20
You're not passing the compatible string and not using
of_platform_populate() so I'm struggling to see how it would work
properly.

=20
sky81452-bl and regulator-sky81452 is parsing the information
in regulator node of its parent node. So I thought these 2 drivers
don't need compatible attribute. That is what it didn't have
compatible string.
Is is mandatory that all drivers should have compatible attribute?

How do they obtain their DT nodes?

[...]

+ return mfd_add_devices(dev, -1, cells, ARRAY_SIZE(cells),
+ NULL, 0, NULL);

=20
This doesn't really need to be in a function of its own. Please pu=

Post by Lee Jones
it in .probe(). Also check for the return value and present the us=

Post by Lee Jones
with an error message if it fails.

=20
I think this need to be, in case of !CONFIG_OF.
Can you please explain more in details?

Then how to you obtain the shared register map you created?

[...]

--=20
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org =E2=94=82 Open source software for ARM SoCs
=46ollow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" i=
n
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Gyungoh Yoo

2014-08-27 04:06:29 UTC

Post by Lee Jones
When you send patch-sets, you should send them connected to one
another AKA threaded. That way, when we're reviewing we can look=

Post by Lee Jones
the other patches in the set for reference. See the man page for=

`git

Post by Lee Jones
send-email` for details.
=20
<commit log>
=20

---

=20
[...]
=20

=20
Have you tested this with DT?
=20
You're not passing the compatible string and not using
of_platform_populate() so I'm struggling to see how it would work
properly.

=20
How do they obtain their DT nodes?

The backlight driver which is one of the child driver is obtain its DT =
node like this

np =3D of_get_child_by_name(dev->parent->of_node, "backlight");

Post by Lee Jones
=20
[...]
=20

+ return mfd_add_devices(dev, -1, cells, ARRAY_SIZE(cells),
+ NULL, 0, NULL);

=20
This doesn't really need to be in a function of its own. Please =

put

Post by Lee Jones
it in .probe(). Also check for the return value and present the =

user

Post by Lee Jones
with an error message if it fails.

=20
I think this need to be, in case of !CONFIG_OF.
Can you please explain more in details?

=20
Then how to you obtain the shared register map you created?

regmap is stored in driver data in MFD.

i2c_set_clientdata(client, regmap);

The child drivers obain the regmap from the parent.

struct regmap *regmap =3D dev_get_drvdata(dev->parent);

Post by Lee Jones
=20
[...]
=20
--=20
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org =E2=94=82 Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

--
To unsubscribe from this list: send the line "unsubscribe devicetree" i=
n
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Lee Jones

2014-08-27 08:39:21 UTC

Post by Lee Jones
When you send patch-sets, you should send them connected to one
another AKA threaded. That way, when we're reviewing we can lo=

ok at

Post by Lee Jones
the other patches in the set for reference. See the man page f=

or `git

Post by Lee Jones
send-email` for details.
=20
<commit log>
=20

---

=20
[...]
=20

=20
Have you tested this with DT?
=20
You're not passing the compatible string and not using
of_platform_populate() so I'm struggling to see how it would wo=

Post by Lee Jones
properly.

=20
sky81452-bl and regulator-sky81452 is parsing the information
in regulator node of its parent node. So I thought these 2 driver=

Post by Gyungoh Yoo
don't need compatible attribute. That is what it didn't have
compatible string.
Is is mandatory that all drivers should have compatible attribute=

Post by Lee Jones
=20
How do they obtain their DT nodes?

=20
The backlight driver which is one of the child driver is obtain its D=

T node like this

=20
np =3D of_get_child_by_name(dev->parent->of_node, "backlight");

The MFD core provides infrastructure so you don't have to do this.

Just place the compatible string in 'struct mfd_cell cells[]' and the
core will match and populate dev->of_node for you.

Post by Lee Jones
[...]
=20

+ return mfd_add_devices(dev, -1, cells, ARRAY_SIZE(cells),
+ NULL, 0, NULL);

=20
This doesn't really need to be in a function of its own. Pleas=

e put

Post by Lee Jones
it in .probe(). Also check for the return value and present th=

e user

Post by Lee Jones
with an error message if it fails.

=20
I think this need to be, in case of !CONFIG_OF.
Can you please explain more in details?

=20
Then how to you obtain the shared register map you created?

=20
regmap is stored in driver data in MFD.
=20
i2c_set_clientdata(client, regmap);
=20
The child drivers obain the regmap from the parent.
=20
struct regmap *regmap =3D dev_get_drvdata(dev->parent);

Ah yes, of course you do. Silly of me to miss this.

I also just noticed that you're also manually populating the
chlidren's platform data. It's easier if you do this from the child
device drivers:

const struct sky81452_platform_data ppdata =3D dev_get_platdata(dev->=
parent);
const struct sky81452_bl_platform_data =3D pdata =3D ppdata->bl_pdata=
;

--=20
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org =E2=94=82 Open source software for ARM SoCs
=46ollow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" i=
n
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Gyungoh Yoo

2014-08-28 01:29:26 UTC

When you send patch-sets, you should send them connected to o=

another AKA threaded. That way, when we're reviewing we can =

look at

the other patches in the set for reference. See the man page=

for `git

send-email` for details.
=20
<commit log>
=20

---

=20
[...]
=20

=20
Have you tested this with DT?
=20
You're not passing the compatible string and not using
of_platform_populate() so I'm struggling to see how it would =

work

properly.

=20
sky81452-bl and regulator-sky81452 is parsing the information
in regulator node of its parent node. So I thought these 2 driv=

ers

Post by Gyungoh Yoo
don't need compatible attribute. That is what it didn't have
compatible string.
Is is mandatory that all drivers should have compatible attribu=

te?

Post by Lee Jones
=20
How do they obtain their DT nodes?

=20
The backlight driver which is one of the child driver is obtain its=

DT node like this

=20
np =3D of_get_child_by_name(dev->parent->of_node, "backlight");

=20
The MFD core provides infrastructure so you don't have to do this.
=20
Just place the compatible string in 'struct mfd_cell cells[]' and the
core will match and populate dev->of_node for you.

I see. Thank you.

Post by Lee Jones
[...]
=20

+ return mfd_add_devices(dev, -1, cells, ARRAY_SIZE(cells),
+ NULL, 0, NULL);

=20
This doesn't really need to be in a function of its own. Ple=

ase put

it in .probe(). Also check for the return value and present =

the user

with an error message if it fails.

=20
I think this need to be, in case of !CONFIG_OF.
Can you please explain more in details?

=20
Then how to you obtain the shared register map you created?

=20
regmap is stored in driver data in MFD.
=20
i2c_set_clientdata(client, regmap);
=20
The child drivers obain the regmap from the parent.
=20
struct regmap *regmap =3D dev_get_drvdata(dev->parent);

=20
Ah yes, of course you do. Silly of me to miss this.
=20
I also just noticed that you're also manually populating the
chlidren's platform data. It's easier if you do this from the child
=20
const struct sky81452_platform_data ppdata =3D dev_get_platdata(dev=

->parent);

Post by Lee Jones
const struct sky81452_bl_platform_data =3D pdata =3D ppdata->bl_pda=

ta;

I think it could be a good way for this. Thank you.

Post by Lee Jones
=20
--=20
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org =E2=94=82 Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Gyungoh Yoo

2014-08-28 10:38:27 UTC

This patch set includes regulator and backlight driver for SKY81452.
Also it includes documents for device tree and module.

v2:
Split the patches for each sub-system.
Added 'reg' attribute for I2C address in device tree documents
Added 'compatible' attribute in child drivers
Renamed CONFIG_SKY81452 to CONFIG_MFD_SKY81452
Changed the dependency from I2C=y to I2C, for CONFIG_MFD_SKY81452
Added message for exception or errors.

Gyungoh Yoo (7):
Adding Skyworks SKY81452 MFD driver
Adding Skyworks SKY81452 regulator driver
Adding Skyworks SKY81452 backlight driver
Adding SKY81452 MFD device tree bindings document
Adding SKY81452 regulator device tree bindings document
Adding SKY81452 backlight device tree bindings document
Adding SKY81452 backlight document

Documentation/backlight/sky81452-backlight.txt | 26 ++
.../devicetree/bindings/i2c/trivial-devices.txt | 1 +
Documentation/devicetree/bindings/mfd/sky81452.txt | 32 ++
.../bindings/regulator/sky81452-regulator.txt | 18 ++
.../devicetree/bindings/vendor-prefixes.txt | 1 +
.../video/backlight/sky81452-backlight.txt | 24 ++
drivers/mfd/Kconfig | 12 +
drivers/mfd/Makefile | 1 +
drivers/mfd/sky81452.c | 108 +++++++
drivers/regulator/Kconfig | 11 +
drivers/regulator/Makefile | 1 +
drivers/regulator/sky81452-regulator.c | 140 +++++++++
drivers/video/backlight/Kconfig | 10 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/sky81452-backlight.c | 347 +++++++++++++++++++++
include/linux/mfd/sky81452.h | 32 ++
include/linux/sky81452-backlight.h | 47 +++
17 files changed, 812 insertions(+)
create mode 100644 Documentation/backlight/sky81452-backlight.txt
create mode 100644 Documentation/devicetree/bindings/mfd/sky81452.txt
create mode 100644 Documentation/devicetree/bindings/regulator/sky81452-regulator.txt
create mode 100644 Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt
create mode 100644 drivers/mfd/sky81452.c
create mode 100644 drivers/regulator/sky81452-regulator.c
create mode 100644 drivers/video/backlight/sky81452-backlight.c
create mode 100644 include/linux/mfd/sky81452.h
create mode 100644 include/linux/sky81452-backlight.h

--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Gyungoh Yoo

2014-08-28 10:43:51 UTC

v2:
Renamed CONFIG_SKY81452 to CONFIG_MFD_SKY81452
Changed the dependency from I2C=y to I2C, for CONFIG_MFD_SKY81452
Added message for exception or errors.

---
drivers/mfd/Kconfig | 12 +++++
drivers/mfd/Makefile | 1 +
drivers/mfd/sky81452.c | 108 +++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/sky81452.h | 32 +++++++++++++
4 files changed, 153 insertions(+)
create mode 100644 drivers/mfd/sky81452.c
create mode 100644 include/linux/mfd/sky81452.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de5abf2..6962b4e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -626,6 +626,18 @@ config MFD_SM501_GPIO
lines on the SM501. The platform data is used to supply the
base number for the first GPIO line to register.

+config MFD_SKY81452
+ tristate "Skyworks Solutions SKY81452"
+ select MFD_CORE
+ select REGMAP_I2C
+ depends on I2C
+ help
+ This is the core driver for the Skyworks SKY81452 backlight and
+ voltage regulator device.
+
+ This driver can also be built as a module. If so, the module
+ will be called sky81452.
+
config MFD_SMSC
bool "SMSC ECE1099 series chips"
depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f001487..6c2f317 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -169,6 +169,7 @@ obj-$(CONFIG_MFD_AS3711) += as3711.o
obj-$(CONFIG_MFD_AS3722) += as3722.o
obj-$(CONFIG_MFD_STW481X) += stw481x.o
obj-$(CONFIG_MFD_IPAQ_MICRO) += ipaq-micro.o
+obj-$(CONFIG_MFD_SKY81452) += sky81452.o

intel-soc-pmic-objs := intel_soc_pmic_core.o intel_soc_pmic_crc.o
obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
diff --git a/drivers/mfd/sky81452.c b/drivers/mfd/sky81452.c
new file mode 100644
index 0000000..c0ca812
--- /dev/null
+++ b/drivers/mfd/sky81452.c
@@ -0,0 +1,108 @@
+/*
+ * sky81452.c SKY81452 MFD driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo <jack.yoo-***@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/sky81452.h>
+
+static const struct regmap_config sky81452_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+};
+
+static int sky81452_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &client->dev;
+ const struct sky81452_platform_data *pdata = dev_get_platdata(dev);
+ struct mfd_cell cells[2];
+ struct regmap *regmap;
+ int ret;
+
+ if (!pdata) {
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+ }
+
+ regmap = devm_regmap_init_i2c(client, &sky81452_config);
+ if (IS_ERR(regmap)) {
+ dev_err(dev, "failed to initialize I2C:%d", PTR_ERR(regmap));
+ return PTR_ERR(regmap);
+ }
+
+ i2c_set_clientdata(client, regmap);
+
+ cells[0].name = "sky81452-bl";
+ cells[0].platform_data = pdata->bl_pdata;
+ cells[0].pdata_size = sizeof(*pdata->bl_pdata);
+ cells[1].name = "sky81452-regulator";
+ cells[1].platform_data = pdata->regulator_init_data;
+ cells[1].pdata_size = sizeof(*pdata->regulator_init_data);
+
+ ret = mfd_add_devices(dev, -1, cells, ARRAY_SIZE(cells), NULL, 0, NULL);
+ if (IS_ERR_VALUE(ret))
+ dev_err(dev, "failed to add child device:%d", ret);
+
+ return ret;
+}
+
+static int sky81452_remove(struct i2c_client *client)
+{
+ mfd_remove_devices(&client->dev);
+ return 0;
+}
+
+static const struct i2c_device_id sky81452_ids[] = {
+ { "sky81452" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, sky81452_ids);
+
+#ifdef CONFIG_OF
+static const struct of_device_id sky81452_of_match[] = {
+ { .compatible = "skyworks,sky81452", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sky81452_of_match);
+#endif
+
+static struct i2c_driver sky81452_driver = {
+ .driver = {
+ .name = "sky81452",
+ .of_match_table = of_match_ptr(sky81452_of_match),
+ },
+ .probe = sky81452_probe,
+ .remove = sky81452_remove,
+ .id_table = sky81452_ids,
+};
+
+module_i2c_driver(sky81452_driver);
+
+MODULE_DESCRIPTION("Skyworks SKY81452 MFD driver");
+MODULE_AUTHOR("Gyungoh Yoo <jack.yoo-***@public.gmane.org>");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("1.1");
diff --git a/include/linux/mfd/sky81452.h b/include/linux/mfd/sky81452.h
new file mode 100644
index 0000000..142f762
--- /dev/null
+++ b/include/linux/mfd/sky81452.h
@@ -0,0 +1,32 @@
+/*
+ * sky81452.h SKY81452 MFD driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo <jack.yoo-***@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _SKY81452_H
+#define _SKY81452_H
+
+#include <linux/sky81452-backlight.h>
+#include <linux/regulator/machine.h>
+
+struct sky81452_platform_data {
+ struct sky81452_bl_platform_data *bl_pdata;
+ struct regulator_init_data *regulator_init_data;
+};
+
+#endif

Gyungoh Yoo

2014-08-29 02:03:21 UTC

v2.1:
Clear mfd_cells to zero before setting.
Added of_compatible into mfd_cells

---
drivers/mfd/Kconfig | 12 +++++
drivers/mfd/Makefile | 1 +
drivers/mfd/sky81452.c | 111 +++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/sky81452.h | 32 +++++++++++++
4 files changed, 156 insertions(+)
create mode 100644 drivers/mfd/sky81452.c
create mode 100644 include/linux/mfd/sky81452.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de5abf2..6962b4e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -626,6 +626,18 @@ config MFD_SM501_GPIO
lines on the SM501. The platform data is used to supply the
base number for the first GPIO line to register.

+config MFD_SKY81452
+ tristate "Skyworks Solutions SKY81452"
+ select MFD_CORE
+ select REGMAP_I2C
+ depends on I2C
+ help
+ This is the core driver for the Skyworks SKY81452 backlight and
+ voltage regulator device.
+
+ This driver can also be built as a module. If so, the module
+ will be called sky81452.
+
config MFD_SMSC
bool "SMSC ECE1099 series chips"
depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f001487..6c2f317 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -169,6 +169,7 @@ obj-$(CONFIG_MFD_AS3711) += as3711.o
obj-$(CONFIG_MFD_AS3722) += as3722.o
obj-$(CONFIG_MFD_STW481X) += stw481x.o
obj-$(CONFIG_MFD_IPAQ_MICRO) += ipaq-micro.o
+obj-$(CONFIG_MFD_SKY81452) += sky81452.o

intel-soc-pmic-objs := intel_soc_pmic_core.o intel_soc_pmic_crc.o
obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
diff --git a/drivers/mfd/sky81452.c b/drivers/mfd/sky81452.c
new file mode 100644
index 0000000..e91b928
--- /dev/null
+++ b/drivers/mfd/sky81452.c
@@ -0,0 +1,111 @@
+/*
+ * sky81452.c SKY81452 MFD driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo <jack.yoo-***@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/sky81452.h>
+
+static const struct regmap_config sky81452_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+};
+
+static int sky81452_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &client->dev;
+ const struct sky81452_platform_data *pdata = dev_get_platdata(dev);
+ struct mfd_cell cells[2];
+ struct regmap *regmap;
+ int ret;
+
+ if (!pdata) {
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+ }
+
+ regmap = devm_regmap_init_i2c(client, &sky81452_config);
+ if (IS_ERR(regmap)) {
+ dev_err(dev, "failed to initialize I2C:%d", PTR_ERR(regmap));
+ return PTR_ERR(regmap);
+ }
+
+ i2c_set_clientdata(client, regmap);
+
+ memset(cells, 0, sizeof(cells));
+ cells[0].name = "sky81452-bl";
+ cells[0].of_compatible = "skyworks,sky81452-backlight";
+ cells[0].platform_data = pdata->bl_pdata;
+ cells[0].pdata_size = sizeof(*pdata->bl_pdata);
+ cells[1].name = "sky81452-regulator";
+ cells[1].of_compatible = "skyworks,sky81452-regulator";
+ cells[1].platform_data = pdata->regulator_init_data;
+ cells[1].pdata_size = sizeof(*pdata->regulator_init_data);
+
+ ret = mfd_add_devices(dev, -1, cells, ARRAY_SIZE(cells), NULL, 0, NULL);
+ if (IS_ERR_VALUE(ret))
+ dev_err(dev, "failed to add child device:%d", ret);
+
+ return ret;
+}
+
+static int sky81452_remove(struct i2c_client *client)
+{
+ mfd_remove_devices(&client->dev);
+ return 0;
+}
+
+static const struct i2c_device_id sky81452_ids[] = {
+ { "sky81452" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, sky81452_ids);
+
+#ifdef CONFIG_OF
+static const struct of_device_id sky81452_of_match[] = {
+ { .compatible = "skyworks,sky81452", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sky81452_of_match);
+#endif
+
+static struct i2c_driver sky81452_driver = {
+ .driver = {
+ .name = "sky81452",
+ .of_match_table = of_match_ptr(sky81452_of_match),
+ },
+ .probe = sky81452_probe,
+ .remove = sky81452_remove,
+ .id_table = sky81452_ids,
+};
+
+module_i2c_driver(sky81452_driver);
+
+MODULE_DESCRIPTION("Skyworks SKY81452 MFD driver");
+MODULE_AUTHOR("Gyungoh Yoo <jack.yoo-***@public.gmane.org>");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("1.1");
diff --git a/include/linux/mfd/sky81452.h b/include/linux/mfd/sky81452.h
new file mode 100644
index 0000000..142f762
--- /dev/null
+++ b/include/linux/mfd/sky81452.h
@@ -0,0 +1,32 @@
+/*
+ * sky81452.h SKY81452 MFD driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo <jack.yoo-***@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _SKY81452_H
+#define _SKY81452_H
+
+#include <linux/sky81452-backlight.h>
+#include <linux/regulator/machine.h>
+
+struct sky81452_platform_data {
+ struct sky81452_bl_platform_data *bl_pdata;
+ struct regulator_init_data *regulator_init_data;
+};
+
+#endif

Gyungoh Yoo

2014-08-28 10:46:37 UTC

v2:
Added 'compatible' attribute in the driver
Added message for exception or errors.

---
drivers/regulator/Kconfig | 11 +++
drivers/regulator/Makefile | 1 +
drivers/regulator/sky81452-regulator.c | 140 +++++++++++++++++++++++++++++++++
3 files changed, 152 insertions(+)
create mode 100644 drivers/regulator/sky81452-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 2dc8289..2b05964 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -483,6 +483,17 @@ config REGULATOR_S5M8767
via I2C bus. S5M8767A have 9 Bucks and 28 LDOs output and
supports DVS mode with 8bits of output voltage control.

+config REGULATOR_SKY81452
+ tristate "Skyworks Solutions SKY81452 voltage regulator"
+ depends on MFD_SKY81452
+ help
+ This driver supports Skyworks SKY81452 voltage output regulator
+ via I2C bus. SKY81452 has one voltage linear regulator can be
+ programmed from 4.5V to 20V.
+
+ This driver can also be built as a module. If so, the module
+ will be called sky81452-regulator.
+
config REGULATOR_ST_PWM
tristate "STMicroelectronics PWM voltage regulator"
depends on ARCH_STI
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index aa4a6aa..d8206ec 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_REGULATOR_RC5T583) += rc5t583-regulator.o
obj-$(CONFIG_REGULATOR_S2MPA01) += s2mpa01.o
obj-$(CONFIG_REGULATOR_S2MPS11) += s2mps11.o
obj-$(CONFIG_REGULATOR_S5M8767) += s5m8767.o
+obj-$(CONFIG_REGULATOR_SKY81452) += sky81452-regulator.o
obj-$(CONFIG_REGULATOR_ST_PWM) += st-pwm.o
obj-$(CONFIG_REGULATOR_STW481X_VMMC) += stw481x-vmmc.o
obj-$(CONFIG_REGULATOR_TI_ABB) += ti-abb-regulator.o
diff --git a/drivers/regulator/sky81452-regulator.c b/drivers/regulator/sky81452-regulator.c
new file mode 100644
index 0000000..b9c54c6
--- /dev/null
+++ b/drivers/regulator/sky81452-regulator.c
@@ -0,0 +1,140 @@
+/*
+ * sky81452-regulator.c SKY81452 regulator driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo <jack.yoo-***@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+
+/* registers */
+#define SKY81452_REG1 0x01
+#define SKY81452_REG3 0x03
+
+/* bit mask */
+#define SKY81452_LEN 0x40
+#define SKY81452_LOUT 0x1F
+
+static struct regulator_ops sky81452_reg_ops = {
+ .list_voltage = regulator_list_voltage_linear_range,
+ .map_voltage = regulator_map_voltage_linear_range,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+};
+
+static const struct regulator_linear_range sky81452_reg_ranges[] = {
+ REGULATOR_LINEAR_RANGE(4500000, 0, 14, 250000),
+ REGULATOR_LINEAR_RANGE(9000000, 15, 31, 1000000),
+};
+
+static const struct regulator_desc sky81452_reg = {
+ .name = "LOUT",
+ .ops = &sky81452_reg_ops,
+ .type = REGULATOR_VOLTAGE,
+ .owner = THIS_MODULE,
+ .n_voltages = SKY81452_LOUT + 1,
+ .linear_ranges = sky81452_reg_ranges,
+ .n_linear_ranges = ARRAY_SIZE(sky81452_reg_ranges),
+ .vsel_reg = SKY81452_REG3,
+ .vsel_mask = SKY81452_LOUT,
+ .enable_reg = SKY81452_REG1,
+ .enable_mask = SKY81452_LEN,
+};
+
+#ifdef CONFIG_OF
+static struct regulator_init_data *sky81452_reg_parse_dt(struct device *dev)
+{
+ struct regulator_init_data *init_data;
+ struct device_node *np = of_node_get(dev->of_node);
+
+ if (!np) {
+ dev_err(dev, "regulator node not found");
+ return ERR_PTR(-ENODATA);
+ }
+
+ init_data = of_get_regulator_init_data(dev, np);
+
+ of_node_put(np);
+ return init_data;
+}
+#else
+static struct regulator_init_data *sky81452_reg_parse_dt(struct device *dev)
+{
+ return ERR_PTR(-EINVAL);
+}
+#endif
+
+static int sky81452_reg_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ const struct regulator_init_data *init_data = dev_get_platdata(dev);
+ struct regulator_config config = { };
+ struct regulator_dev *rdev;
+
+ if (!init_data) {
+ init_data = sky81452_reg_parse_dt(dev);
+ if (IS_ERR(init_data))
+ return PTR_ERR(init_data);
+ }
+
+ config.dev = dev;
+ config.init_data = init_data;
+ config.of_node = dev->of_node;
+ config.regmap = dev_get_drvdata(dev->parent);
+
+ rdev = devm_regulator_register(dev, &sky81452_reg, &config);
+ if (IS_ERR(rdev)) {
+ dev_err(dev, "failed to register regulator:%d", PTR_ERR(rdev));
+ return PTR_ERR(rdev);
+ }
+
+ platform_set_drvdata(pdev, rdev);
+
+ return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id sky81452_reg_of_match[] = {
+ { .compatible = "skyworks,sky81452-regulator", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sky81452_reg_of_match);
+#endif
+
+static struct platform_driver sky81452_reg_driver = {
+ .driver = {
+ .name = "sky81452-regulator",
+ .of_match_table = of_match_ptr(sky81452_reg_of_match),
+ },
+ .probe = sky81452_reg_probe,
+};
+
+module_platform_driver(sky81452_reg_driver);
+
+MODULE_DESCRIPTION("Skyworks SKY81452 Regulator driver");
+MODULE_AUTHOR("Gyungoh Yoo <jack.yoo-***@public.gmane.org>");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("1.1");

Mark Brown

2014-08-28 11:02:54 UTC

Post by Gyungoh Yoo
Added 'compatible' attribute in the driver
Added message for exception or errors.

This driver has already been applied, please send any changes as
incremental patches against current code. I would *not* expect to see
a compatible being defined for a MFD function device though unless it's
genuinely reusable.

Gyungoh Yoo

2014-08-29 02:21:55 UTC

Post by Gyungoh Yoo
Added 'compatible' attribute in the driver
Added message for exception or errors.

I am really sorry about confusing. I will resumit this regulator driver as the incremental patches.

BTW,
as Lee Jones-MFD supporter mentioned,
MFD core provides infrastructure for child drivers using compatible.
https://lkml.org/lkml/2014/8/27/132

I want your opinion.

Thank you.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Gyungoh Yoo

2014-08-28 10:49:50 UTC

v2:
Added 'compatible' attribute in child drivers
Added messages for exception or errors.

---
drivers/video/backlight/Kconfig | 10 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/sky81452-backlight.c | 347 +++++++++++++++++++++++++++
include/linux/sky81452-backlight.h | 47 ++++
4 files changed, 405 insertions(+)
create mode 100644 drivers/video/backlight/sky81452-backlight.c
create mode 100644 include/linux/sky81452-backlight.h

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 8d03924..2586fdd 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -409,6 +409,16 @@ config BACKLIGHT_PANDORA
If you have a Pandora console, say Y to enable the
backlight driver.

+config BACKLIGHT_SKY81452
+ tristate "Backlight driver for SKY81452"
+ depends on BACKLIGHT_CLASS_DEVICE && MFD_SKY81452
+ help
+ If you have a Skyworks SKY81452, say Y to enable the
+ backlight driver.
+
+ To compile this driver as a module, choose M here: the module will
+ be called sky81452-backlight
+
config BACKLIGHT_TPS65217
tristate "TPS65217 Backlight"
depends on BACKLIGHT_CLASS_DEVICE && MFD_TPS65217
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index fcd50b73..d67073f 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_BACKLIGHT_PANDORA) += pandora_bl.o
obj-$(CONFIG_BACKLIGHT_PCF50633) += pcf50633-backlight.o
obj-$(CONFIG_BACKLIGHT_PWM) += pwm_bl.o
obj-$(CONFIG_BACKLIGHT_SAHARA) += kb3886_bl.o
+obj-$(CONFIG_BACKLIGHT_SKY81452) += sky81452-backlight.o
obj-$(CONFIG_BACKLIGHT_TOSA) += tosa_bl.o
obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o
obj-$(CONFIG_BACKLIGHT_WM831X) += wm831x_bl.o
diff --git a/drivers/video/backlight/sky81452-backlight.c b/drivers/video/backlight/sky81452-backlight.c
new file mode 100644
index 0000000..d292026
--- /dev/null
+++ b/drivers/video/backlight/sky81452-backlight.c
@@ -0,0 +1,347 @@
+/*
+ * sky81452-backlight.c SKY81452 backlight driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo <jack.yoo-***@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/regmap.h>
+#include <linux/backlight.h>
+#include <linux/sky81452-backlight.h>
+
+/* registers */
+#define SKY81452_REG0 0x00
+#define SKY81452_REG1 0x01
+#define SKY81452_REG2 0x02
+#define SKY81452_REG4 0x04
+#define SKY81452_REG5 0x05
+
+/* bit mask */
+#define SKY81452_CS 0xFF
+#define SKY81452_EN 0x3F
+#define SKY81452_IGPW 0x20
+#define SKY81452_PWMMD 0x10
+#define SKY81452_PHASE 0x08
+#define SKY81452_ILIM 0x04
+#define SKY81452_VSHRT 0x03
+#define SKY81452_OCP 0x80
+#define SKY81452_OTMP 0x40
+#define SKY81452_SHRT 0x3F
+#define SKY81452_OPN 0x3F
+
+#define SKY81452_DEFAULT_NAME "lcd-backlight"
+#define SKY81452_MAX_BRIGHTNESS (SKY81452_CS + 1)
+
+#define CTZ(b) __builtin_ctz(b)
+
+static int sky81452_bl_update_status(struct backlight_device *bd)
+{
+ const struct sky81452_bl_platform_data *pdata =
+ dev_get_platdata(bd->dev.parent);
+ const unsigned int brightness = (unsigned int)bd->props.brightness;
+ struct regmap *regmap = bl_get_data(bd);
+ int ret;
+
+ if (brightness > 0) {
+ ret = regmap_write(regmap, SKY81452_REG0, brightness - 1);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ return regmap_update_bits(regmap, SKY81452_REG1, SKY81452_EN,
+ pdata->enable << CTZ(SKY81452_EN));
+ }
+
+ return regmap_update_bits(regmap, SKY81452_REG1, SKY81452_EN, 0);
+}
+
+static int sky81452_bl_get_brightness(struct backlight_device *bd)
+{
+ return bd->props.brightness;
+}
+
+static const struct backlight_ops sky81452_bl_ops = {
+ .update_status = sky81452_bl_update_status,
+ .get_brightness = sky81452_bl_get_brightness,
+};
+
+static ssize_t sky81452_bl_store_enable(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct regmap *regmap = bl_get_data(to_backlight_device(dev));
+ unsigned long value;
+ int ret;
+
+ ret = kstrtoul(buf, 16, &value);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ ret = regmap_update_bits(regmap, SKY81452_REG1, SKY81452_EN,
+ value << CTZ(SKY81452_EN));
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ return count;
+}
+
+static ssize_t sky81452_bl_show_open_short(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct regmap *regmap = bl_get_data(to_backlight_device(dev));
+ unsigned int reg, value = 0;
+ char tmp[3];
+ int i, ret;
+
+ reg = !strcmp(attr->attr.name, "open") ? SKY81452_REG5 : SKY81452_REG4;
+ ret = regmap_read(regmap, reg, &value);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ if (value & SKY81452_SHRT) {
+ *buf = 0;
+ for (i = 0; i < 6; i++) {
+ if (value & 0x01) {
+ sprintf(tmp, "%d ", i + 1);
+ strcat(buf, tmp);
+ }
+ value >>= 1;
+ }
+ strcat(buf, "\n");
+ } else
+ strcpy(buf, "none\n");
+
+ return strlen(buf);
+}
+
+static ssize_t sky81452_bl_show_fault(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct regmap *regmap = bl_get_data(to_backlight_device(dev));
+ unsigned int value = 0;
+ int ret;
+
+ ret = regmap_read(regmap, SKY81452_REG4, &value);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ *buf = 0;
+
+ if (value & SKY81452_OCP)
+ strcat(buf, "over-current ");
+
+ if (value & SKY81452_OTMP)
+ strcat(buf, "over-temperature");
+
+ strcat(buf, "\n");
+ return strlen(buf);
+}
+
+static DEVICE_ATTR(enable, S_IWGRP | S_IWUSR, NULL, sky81452_bl_store_enable);
+static DEVICE_ATTR(open, S_IRUGO, sky81452_bl_show_open_short, NULL);
+static DEVICE_ATTR(short, S_IRUGO, sky81452_bl_show_open_short, NULL);
+static DEVICE_ATTR(fault, S_IRUGO, sky81452_bl_show_fault, NULL);
+
+static struct attribute *sky81452_bl_attribute[] = {
+ &dev_attr_enable.attr,
+ &dev_attr_open.attr,
+ &dev_attr_short.attr,
+ &dev_attr_fault.attr,
+ NULL
+};
+
+static const struct attribute_group sky81452_bl_attr_group = {
+ .attrs = sky81452_bl_attribute,
+};
+
+#ifdef CONFIG_OF
+static struct sky81452_bl_platform_data *sky81452_bl_parse_dt(
+ struct device *dev)
+{
+ struct device_node *np = of_node_get(dev->of_node);
+ struct sky81452_bl_platform_data *pdata;
+ int ret;
+
+ if (!np) {
+ dev_err(dev, "backlight node not found");
+ return ERR_PTR(-ENODATA);
+ }
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata) {
+ of_node_put(np);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ of_property_read_string(np, "name", &pdata->name);
+ pdata->ignore_pwm = of_property_read_bool(np, "ignore-pwm");
+ pdata->dpwm_mode = of_property_read_bool(np, "dpwm-mode");
+ pdata->phase_shift = of_property_read_bool(np, "phase-shift");
+
+ pdata->gpio_enable = of_get_named_gpio(np, "gpio-enable", 0);
+ if (IS_ERR_VALUE(pdata->gpio_enable))
+ pdata->gpio_enable = -1;
+
+ ret = of_property_read_u32(np, "enable", &pdata->enable);
+ if (IS_ERR_VALUE(ret))
+ pdata->enable = SKY81452_EN >> CTZ(SKY81452_EN);
+
+ ret = of_property_read_u32(np, "short-detection-threshold",
+ &pdata->short_detection_threshold);
+ if (IS_ERR_VALUE(ret))
+ pdata->short_detection_threshold = 7;
+
+ ret = of_property_read_u32(np, "boost-current-limit",
+ &pdata->boost_current_limit);
+ if (IS_ERR_VALUE(ret))
+ pdata->boost_current_limit = 2750;
+
+ of_node_put(np);
+ return pdata;
+}
+#else
+static struct sky81452_bl_platform_data *sky81452_bl_parse_dt(
+ struct device *dev)
+{
+ return ERR_PTR(-EINVAL);
+}
+#endif
+
+static int sky81452_bl_init_device(struct regmap *regmap,
+ struct sky81452_bl_platform_data *pdata)
+{
+ unsigned int value;
+
+ value = pdata->ignore_pwm ? SKY81452_IGPW : 0;
+ value |= pdata->dpwm_mode ? SKY81452_PWMMD : 0;
+ value |= pdata->phase_shift ? 0 : SKY81452_PHASE;
+
+ if (pdata->boost_current_limit == 2300)
+ value |= SKY81452_ILIM;
+ else if (pdata->boost_current_limit != 2720)
+ return -EINVAL;
+
+ if (pdata->short_detection_threshold < 4 ||
+ pdata->short_detection_threshold > 7)
+ return -EINVAL;
+ value |= (7 - pdata->short_detection_threshold) << CTZ(SKY81452_VSHRT);
+
+ return regmap_write(regmap, SKY81452_REG2, value);
+}
+
+static int sky81452_bl_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct regmap *regmap = dev_get_drvdata(dev->parent);
+ struct sky81452_bl_platform_data *pdata = dev_get_platdata(dev);
+ struct backlight_device *bd;
+ struct backlight_properties props;
+ const char *name;
+ int ret;
+
+ if (!pdata) {
+ pdata = sky81452_bl_parse_dt(dev);
+ if (IS_ERR(pdata))
+ return PTR_ERR(pdata);
+ }
+
+ if (pdata->gpio_enable >= 0) {
+ ret = devm_gpio_request_one(dev, pdata->gpio_enable,
+ GPIOF_OUT_INIT_HIGH, "sky81452-en");
+ if (IS_ERR_VALUE(ret)) {
+ dev_err(dev, "failed to request GPIO:%d", ret);
+ return ret;
+ }
+ }
+
+ ret = sky81452_bl_init_device(regmap, pdata);
+ if (IS_ERR_VALUE(ret)) {
+ dev_err(dev, "failed to initialize device:%d", ret);
+ return ret;
+ }
+
+ memset(&props, 0, sizeof(props));
+ props.max_brightness = SKY81452_MAX_BRIGHTNESS,
+ name = pdata->name ? pdata->name : SKY81452_DEFAULT_NAME;
+ bd = devm_backlight_device_register(dev, name, dev, regmap,
+ &sky81452_bl_ops, &props);
+ if (IS_ERR(bd)) {
+ dev_err(dev, "failed to register backlight:%d", PTR_ERR(bd));
+ return PTR_ERR(bd);
+ }
+
+ platform_set_drvdata(pdev, bd);
+
+ ret = sysfs_create_group(&bd->dev.kobj, &sky81452_bl_attr_group);
+ if (IS_ERR_VALUE(ret)) {
+ dev_err(dev, "failed to create attribute:%d", ret);
+ goto err;
+ }
+
+ return ret;
+err:
+ backlight_device_unregister(bd);
+ return ret;
+}
+
+static int sky81452_bl_remove(struct platform_device *pdev)
+{
+ const struct sky81452_bl_platform_data *pdata =
+ dev_get_platdata(&pdev->dev);
+ struct backlight_device *bd = platform_get_drvdata(pdev);
+
+ sysfs_remove_group(&bd->dev.kobj, &sky81452_bl_attr_group);
+
+ bd->props.power = FB_BLANK_UNBLANK;
+ bd->props.brightness = 0;
+ backlight_update_status(bd);
+
+ if (pdata->gpio_enable >= 0)
+ gpio_set_value_cansleep(pdata->gpio_enable, 0);
+
+ return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id sky81452_bl_of_match[] = {
+ { .compatible = "skyworks,sky81452-backlight", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sky81452_bl_of_match);
+#endif
+
+static struct platform_driver sky81452_bl_driver = {
+ .driver = {
+ .name = "sky81452-backlight",
+ .of_match_table = of_match_ptr(sky81452_bl_of_match),
+ },
+ .probe = sky81452_bl_probe,
+ .remove = sky81452_bl_remove,
+};
+
+module_platform_driver(sky81452_bl_driver);
+
+MODULE_DESCRIPTION("Skyworks SKY81452 backlight driver");
+MODULE_AUTHOR("Gyungoh Yoo <jack.yoo-***@public.gmane.org>");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("1.1");
diff --git a/include/linux/sky81452-backlight.h b/include/linux/sky81452-backlight.h
new file mode 100644
index 0000000..b3635f9
--- /dev/null
+++ b/include/linux/sky81452-backlight.h
@@ -0,0 +1,47 @@
+/*
+ * sky81452.h SKY81452 backlight driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo <jack.yoo-***@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _SKY81452_BACKLIGHT_H
+#define _SKY81452_BACKLIGHT_H
+
+/**
+ * struct sky81452_platform_data
+ * @name: backlight driver name.
+ If it is not defined, default name is lcd-backlight.
+ * @gpio_enable:GPIO number which control EN pin
+ * @enable: Enable mask for current sink channel 1, 2, 3, 4, 5 and 6.
+ * @ignore_pwm: true if DPWMI should be ignored.
+ * @dpwm_mode: true is DPWM dimming mode, otherwise Analog dimming mode.
+ * @phase_shift:true is phase shift mode.
+ * @short_detecion_threshold: It should be one of 4, 5, 6 and 7V.
+ * @boost_current_limit: It should be one of 2300, 2750mA.
+ */
+struct sky81452_bl_platform_data {
+ const char *name;
+ int gpio_enable;
+ unsigned int enable;
+ bool ignore_pwm;
+ bool dpwm_mode;
+ bool phase_shift;
+ unsigned int short_detection_threshold;
+ unsigned int boost_current_limit;
+};
+
+#endif

Gyungoh Yoo

2014-08-28 10:56:19 UTC

v2:
Added 'reg' attribute for I2C address in device tree documents

---
.../devicetree/bindings/i2c/trivial-devices.txt | 1 +
Documentation/devicetree/bindings/mfd/sky81452.txt | 32 ++++++++++++++++++++++
.../devicetree/bindings/vendor-prefixes.txt | 1 +
3 files changed, 34 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/sky81452.txt

diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index 6af570e..ff77879 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -78,6 +78,7 @@ ramtron,24c64 i2c serial eeprom (24cxx)
ricoh,rs5c372a I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
samsung,24ad0xd1 S524AD0XF1 (128K/256K-bit Serial EEPROM for Low Power)
sii,s35390a 2-wire CMOS real-time clock
+skyworks,sky81452 Skyworks SKY81452: Six-Channel White LED Driver with Touch Panel Bias Supply
st-micro,24c256 i2c serial eeprom (24cxx)
stm,m41t00 Serial Access TIMEKEEPER
stm,m41t62 Serial real-time clock (RTC) with alarm
diff --git a/Documentation/devicetree/bindings/mfd/sky81452.txt b/Documentation/devicetree/bindings/mfd/sky81452.txt
new file mode 100644
index 0000000..d61904a
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/sky81452.txt
@@ -0,0 +1,32 @@
+SKY81452 bindings
+
+Required properties:
+- compatible : Must be "skyworks,sky81452"
+- reg : I2C slave address
+
+Required child nodes:
+- backlight : container node for backlight following the binding
+ in video/backlight/sky81452-backlight.txt
+- regulator : container node for regulators following the binding
+ in regulator/sky81452-regulator.txt
+
+Example:
+
+ ***@2c {
+ compatible = "skyworks,sky81452";
+ reg = <0x2c>;
+
+ backlight {
+ compatible = "skyworks,sky81452-backlight";
+ name = "pwm-backlight";
+ enable = <0x3F>;
+ ignore-pwm;
+ };
+
+ regulator {
+ compatible = "skyworks,sky81452-regulator";
+ regulator-name = "touch_en";
+ regulator-min-microvolt = <4500000>;
+ regulator-max-microvolt = <8000000>;
+ };
+ };
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index ac7269f..c755978 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -125,6 +125,7 @@ silabs Silicon Laboratories
simtek
sii Seiko Instruments, Inc.
sirf SiRF Technology, Inc.
+skyworks Skyworks Solutions, Inc.
smsc Standard Microsystems Corporation
snps Synopsys, Inc.
solidrun SolidRun

--
1.9.1

Gyungoh Yoo

2014-08-28 10:58:38 UTC

v2:
Added 'compatible' attribute in child drivers

---
.../bindings/regulator/sky81452-regulator.txt | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/sky81452-regulator.txt

diff --git a/Documentation/devicetree/bindings/regulator/sky81452-regulator.txt b/Documentation/devicetree/bindings/regulator/sky81452-regulator.txt
new file mode 100644
index 0000000..7b9ff18
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/sky81452-regulator.txt
@@ -0,0 +1,18 @@
+SKY81452 voltage regulator
+
+Required properties:
+- compatible : Must be "skyworks,sky81452-regulator"
+- any required generic properties defined in regulator.txt
+
+Optional properties:
+- any available generic properties defined in regulator.txt
+
+Example:
+
+ regulator {
+ compatible = "skyworks,sky81452-regulator";
+ /* generic regulator properties */
+ regulator-name = "touch_en";
+ regulator-min-microvolt = <4500000>;
+ regulator-max-microvolt = <8000000>;
+ };

Gyungoh Yoo

2014-09-01 02:46:48 UTC

Please ingnore 'only' this patch.

When sky81452-regulator driver was applied before,
The previous version of this document 'sky81452-regulator.txt' was applied.
I will resubmit the changes as incremental patches against current code of sky81452-regulator.txt.

I am sorry for confusiong.
Thank you.

Post by Gyungoh Yoo
Added 'compatible' attribute in child drivers
---
.../bindings/regulator/sky81452-regulator.txt | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/sky81452-regulator.txt
diff --git a/Documentation/devicetree/bindings/regulator/sky81452-regulator.txt b/Documentation/devicetree/bindings/regulator/sky81452-regulator.txt
new file mode 100644
index 0000000..7b9ff18
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/sky81452-regulator.txt
@@ -0,0 +1,18 @@
+SKY81452 voltage regulator
+
+- compatible : Must be "skyworks,sky81452-regulator"
+- any required generic properties defined in regulator.txt
+
+- any available generic properties defined in regulator.txt
+
+
+ regulator {
+ compatible = "skyworks,sky81452-regulator";
+ /* generic regulator properties */
+ regulator-name = "touch_en";
+ regulator-min-microvolt = <4500000>;
+ regulator-max-microvolt = <8000000>;
+ };
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Gyungoh Yoo

2014-08-28 11:00:01 UTC

v2:
Added 'compatible' attribute in child drivers

---
.../video/backlight/sky81452-backlight.txt | 24 ++++++++++++++++++++++
1 file changed, 24 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt

diff --git a/Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt b/Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt
new file mode 100644
index 0000000..2c2d947
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt
@@ -0,0 +1,24 @@
+SKY81452-backlight bindings
+
+Required properties:
+- compatible : Must be "skyworks,sky81452-backlight"
+
+Optional properties:
+- name : Name of backlight device. Default is 'lcd-backlight'.
+- gpio-enable : GPIO to use to EN pin.
+- enable : Enable mask for current sink channel 1 to 6.
+- ignore-pwm : Ignore both PWM input
+- dpwm-mode : Enable DPWM dimming mode, otherwise Analog dimming mode
+- phase-shift : Enable phase shift mode
+- ovp-level : Over-voltage protection level. Should be between 14 or 28V.
+- short-detection-threshold : It should be one of 4, 5, 6 and 7V.
+- boost-current-limit : It should be one of 800, 1100 and 1500mA.
+
+Example:
+
+ backlight {
+ compatible = "skyworks,sky81452-backlight";
+ name = "pwm-backlight";
+ enable = <0x3F>;
+ ignore-pwm;
+ };

Gyungoh Yoo

2014-08-28 11:01:49 UTC

v2:
split the patches

---
Documentation/backlight/sky81452-backlight.txt | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
create mode 100644 Documentation/backlight/sky81452-backlight.txt

diff --git a/Documentation/backlight/sky81452-backlight.txt b/Documentation/backlight/sky81452-backlight.txt
new file mode 100644
index 0000000..39f2f8c
--- /dev/null
+++ b/Documentation/backlight/sky81452-backlight.txt
@@ -0,0 +1,26 @@
+backlight driver for sky81452
+==========================
+
+* Skyworks Solutions SKY81452 :
+ High-Efficiency, Six-Channel White LED Driver with Touch Panel Bias Supply
+
+Description
+-----------
+The SKY81452 is a highly integrated, high-efficiency LED backlight solution for
+tablets, notebook computers, monitors, and other portable devices.
+
+The SKY81452 supports Direct Pulse Width Modulation(DPWM) dimming and Analog
+Pulse Width Modulation (APWM). In the DPWM dimming mode, the output waveform
+follows the Pulse Width Modulation Input(PWMI) signal and the current level is
+set by I2C control and an external RSET resistor. In the APWM mode, the PWMI
+signal duty and the I2C brightness control signal are multiplied to control
+the output current level.
+
+The driver exports some additional attributes for SKY81452 backlight.
+- enable : Current sink channels. Bit0 is for channel 1, Bit1 is for channel 2
+ and Bit2 is for channel 3. In same way, Bit5 is for channel 6.
+ For example, if 5 is written to this attribute, channel 1 and
+ channel 3 are enabled.
+- open : Shows an open LED fault.
+- short : Shows an short LED fault.
+- fault : Shows information about over-current and over-temperature.

--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Gyungoh Yoo

2014-10-07 04:55:45 UTC

This patch set includes regulator and backlight driver for SKY81452.
Also it includes documents for device tree and module.

v3:
Cleaned-up DBG messages.
Cleaned-up DT.

v2:
Split the patches for each sub-system.
Added 'reg' attribute for I2C address in device tree documents
Added 'compatible' attribute in child drivers
Renamed CONFIG_SKY81452 to CONFIG_MFD_SKY81452
Changed the dependency from I2C=y to I2C, for CONFIG_MFD_SKY81452
Added message for exception or errors.

Gyungoh Yoo (8):
mfd: Add support for Skyworks SKY81452 driver
backlight: Add support Skyworks SKY81452 backlight driver
devicetree: mfd: Add new SKY81452 mfd binding
devicetree: backlight: add new SKY81452 backlight binding
devicetree: Add vendor prefix for Skyworks Solutions, Inc.
devicetree: i2c: Add SKY81452 to the Trivial Devices list
regulator: sky81452: Add compatible string for device binding
devicetree: regulator: sky81452: Add compatible string for device
binding

.../devicetree/bindings/i2c/trivial-devices.txt | 1 +
Documentation/devicetree/bindings/mfd/sky81452.txt | 32 ++
.../bindings/regulator/sky81452-regulator.txt | 2 +
.../devicetree/bindings/vendor-prefixes.txt | 1 +
.../video/backlight/sky81452-backlight.txt | 24 ++
drivers/mfd/Kconfig | 12 +
drivers/mfd/Makefile | 1 +
drivers/mfd/sky81452.c | 111 +++++++
drivers/regulator/sky81452-regulator.c | 22 +-
drivers/video/backlight/Kconfig | 10 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/sky81452-backlight.c | 347 +++++++++++++++++++++
include/linux/mfd/sky81452.h | 32 ++
13 files changed, 590 insertions(+), 6 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/sky81452.txt
create mode 100644 Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt
create mode 100644 drivers/mfd/sky81452.c
create mode 100644 drivers/video/backlight/sky81452-backlight.c
create mode 100644 include/linux/mfd/sky81452.h

Gyungoh Yoo

2014-10-07 04:58:21 UTC

Signed-off-by: Gyungoh Yoo <***@skyworksinc.com>
---
drivers/mfd/Kconfig | 12 +++++
drivers/mfd/Makefile | 1 +
drivers/mfd/sky81452.c | 111 +++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/sky81452.h | 32 +++++++++++++
4 files changed, 156 insertions(+)
create mode 100644 drivers/mfd/sky81452.c
create mode 100644 include/linux/mfd/sky81452.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de5abf2..6962b4e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -626,6 +626,18 @@ config MFD_SM501_GPIO
lines on the SM501. The platform data is used to supply the
base number for the first GPIO line to register.

+config MFD_SKY81452
+ tristate "Skyworks Solutions SKY81452"
+ select MFD_CORE
+ select REGMAP_I2C
+ depends on I2C
+ help
+ This is the core driver for the Skyworks SKY81452 backlight and
+ voltage regulator device.
+
+ This driver can also be built as a module. If so, the module
+ will be called sky81452.
+
config MFD_SMSC
bool "SMSC ECE1099 series chips"
depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f001487..6c2f317 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -169,6 +169,7 @@ obj-$(CONFIG_MFD_AS3711) += as3711.o
obj-$(CONFIG_MFD_AS3722) += as3722.o
obj-$(CONFIG_MFD_STW481X) += stw481x.o
obj-$(CONFIG_MFD_IPAQ_MICRO) += ipaq-micro.o
+obj-$(CONFIG_MFD_SKY81452) += sky81452.o

intel-soc-pmic-objs := intel_soc_pmic_core.o intel_soc_pmic_crc.o
obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
diff --git a/drivers/mfd/sky81452.c b/drivers/mfd/sky81452.c
new file mode 100644
index 0000000..9130112
--- /dev/null
+++ b/drivers/mfd/sky81452.c
@@ -0,0 +1,111 @@
+/*
+ * sky81452.c SKY81452 MFD driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo <***@skyworksinc.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/sky81452.h>
+
+static const struct regmap_config sky81452_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+};
+
+static int sky81452_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &client->dev;
+ const struct sky81452_platform_data *pdata = dev_get_platdata(dev);
+ struct mfd_cell cells[2];
+ struct regmap *regmap;
+ int ret;
+
+ if (!pdata) {
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+ }
+
+ regmap = devm_regmap_init_i2c(client, &sky81452_config);
+ if (IS_ERR(regmap)) {
+ dev_err(dev, "failed to initialize. err=%ld", PTR_ERR(regmap));
+ return PTR_ERR(regmap);
+ }
+
+ i2c_set_clientdata(client, regmap);
+
+ memset(cells, 0, sizeof(cells));
+ cells[0].name = "sky81452-backlight";
+ cells[0].of_compatible = "skyworks,sky81452-backlight";
+ cells[0].platform_data = pdata->bl_pdata;
+ cells[0].pdata_size = sizeof(*pdata->bl_pdata);
+ cells[1].name = "sky81452-regulator";
+ cells[1].of_compatible = "skyworks,sky81452-regulator";
+ cells[1].platform_data = pdata->regulator_init_data;
+ cells[1].pdata_size = sizeof(*pdata->regulator_init_data);
+
+ ret = mfd_add_devices(dev, -1, cells, ARRAY_SIZE(cells), NULL, 0, NULL);
+ if (ret)
+ dev_err(dev, "failed to add child devices. err=%d", ret);
+
+ return ret;
+}
+
+static int sky81452_remove(struct i2c_client *client)
+{
+ mfd_remove_devices(&client->dev);
+ return 0;
+}
+
+static const struct i2c_device_id sky81452_ids[] = {
+ { "sky81452" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, sky81452_ids);
+
+#ifdef CONFIG_OF
+static const struct of_device_id sky81452_of_match[] = {
+ { .compatible = "skyworks,sky81452", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sky81452_of_match);
+#endif
+
+static struct i2c_driver sky81452_driver = {
+ .driver = {
+ .name = "sky81452",
+ .of_match_table = of_match_ptr(sky81452_of_match),
+ },
+ .probe = sky81452_probe,
+ .remove = sky81452_remove,
+ .id_table = sky81452_ids,
+};
+
+module_i2c_driver(sky81452_driver);
+
+MODULE_DESCRIPTION("Skyworks SKY81452 MFD driver");
+MODULE_AUTHOR("Gyungoh Yoo <***@skyworksinc.com>");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("1.2");
diff --git a/include/linux/mfd/sky81452.h b/include/linux/mfd/sky81452.h
new file mode 100644
index 0000000..142f762
--- /dev/null
+++ b/include/linux/mfd/sky81452.h
@@ -0,0 +1,32 @@
+/*
+ * sky81452.h SKY81452 MFD driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo <***@skyworksinc.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _SKY81452_H
+#define _SKY81452_H
+
+#include <linux/sky81452-backlight.h>
+#include <linux/regulator/machine.h>
+
+struct sky81452_platform_data {
+ struct sky81452_bl_platform_data *bl_pdata;
+ struct regulator_init_data *regulator_init_data;
+};
+
+#endif

--
1.9.1

Lee Jones

2014-10-07 08:12:00 UTC

You say that this is v3, but there is no change log (usually placed
between the '---' and the diffstat). Also you have 8 patches in this
set with no 0/8 patch explaining what you're trying to achieve. For
this size patch you also require a patch commit log. What does this
driver provide? How does it provide it? Once you've addressed these
issues, I'll review your patches.

Post by Gyungoh Yoo
---
drivers/mfd/Kconfig | 12 +++++
drivers/mfd/Makefile | 1 +
drivers/mfd/sky81452.c | 111 +++++++++++++++++++++++++++++++++=

++++++++++

Post by Gyungoh Yoo
include/linux/mfd/sky81452.h | 32 +++++++++++++
4 files changed, 156 insertions(+)
create mode 100644 drivers/mfd/sky81452.c
create mode 100644 include/linux/mfd/sky81452.h
=20
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de5abf2..6962b4e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -626,6 +626,18 @@ config MFD_SM501_GPIO
lines on the SM501. The platform data is used to supply the
base number for the first GPIO line to register.
=20
+config MFD_SKY81452
+ tristate "Skyworks Solutions SKY81452"
+ select MFD_CORE
+ select REGMAP_I2C
+ depends on I2C
+ help
+ This is the core driver for the Skyworks SKY81452 backlight and
+ voltage regulator device.
+
+ This driver can also be built as a module. If so, the module
+ will be called sky81452.
+
config MFD_SMSC
bool "SMSC ECE1099 series chips"
depends on I2C=3Dy
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f001487..6c2f317 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -169,6 +169,7 @@ obj-$(CONFIG_MFD_AS3711) +=3D as3711.o
obj-$(CONFIG_MFD_AS3722) +=3D as3722.o
obj-$(CONFIG_MFD_STW481X) +=3D stw481x.o
obj-$(CONFIG_MFD_IPAQ_MICRO) +=3D ipaq-micro.o
+obj-$(CONFIG_MFD_SKY81452) +=3D sky81452.o
=20
intel-soc-pmic-objs :=3D intel_soc_pmic_core.o intel_soc_pmic_crc.o
obj-$(CONFIG_INTEL_SOC_PMIC) +=3D intel-soc-pmic.o
diff --git a/drivers/mfd/sky81452.c b/drivers/mfd/sky81452.c
new file mode 100644
index 0000000..9130112
--- /dev/null
+++ b/drivers/mfd/sky81452.c
@@ -0,0 +1,111 @@
+/*
+ * sky81452.c SKY81452 MFD driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ *
+ * This program is free software; you can redistribute it and/or mod=

ify it

Post by Gyungoh Yoo
+ * under the terms of the GNU General Public License as published by=

the

Post by Gyungoh Yoo
+ * Free Software Foundation; either version 2, or (at your option) a=

Post by Gyungoh Yoo
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, b=

Post by Gyungoh Yoo
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License=

along

Post by Gyungoh Yoo
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/sky81452.h>
+
+static const struct regmap_config sky81452_config =3D {
+ .reg_bits =3D 8,
+ .val_bits =3D 8,
+};
+
+static int sky81452_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device *dev =3D &client->dev;
+ const struct sky81452_platform_data *pdata =3D dev_get_platdata(dev=

);

Post by Gyungoh Yoo
+ struct mfd_cell cells[2];
+ struct regmap *regmap;
+ int ret;
+
+ if (!pdata) {
+ pdata =3D devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+ }
+
+ regmap =3D devm_regmap_init_i2c(client, &sky81452_config);
+ if (IS_ERR(regmap)) {
+ dev_err(dev, "failed to initialize. err=3D%ld", PTR_ERR(regmap));
+ return PTR_ERR(regmap);
+ }
+
+ i2c_set_clientdata(client, regmap);
+
+ memset(cells, 0, sizeof(cells));
+ cells[0].name =3D "sky81452-backlight";
+ cells[0].of_compatible =3D "skyworks,sky81452-backlight";
+ cells[0].platform_data =3D pdata->bl_pdata;
+ cells[0].pdata_size =3D sizeof(*pdata->bl_pdata);
+ cells[1].name =3D "sky81452-regulator";
+ cells[1].of_compatible =3D "skyworks,sky81452-regulator";
+ cells[1].platform_data =3D pdata->regulator_init_data;
+ cells[1].pdata_size =3D sizeof(*pdata->regulator_init_data);
+
+ ret =3D mfd_add_devices(dev, -1, cells, ARRAY_SIZE(cells), NULL, 0,=

NULL);

Post by Gyungoh Yoo
+ if (ret)
+ dev_err(dev, "failed to add child devices. err=3D%d", ret);
+
+ return ret;
+}
+
+static int sky81452_remove(struct i2c_client *client)
+{
+ mfd_remove_devices(&client->dev);
+ return 0;
+}
+
+static const struct i2c_device_id sky81452_ids[] =3D {
+ { "sky81452" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, sky81452_ids);
+
+#ifdef CONFIG_OF
+static const struct of_device_id sky81452_of_match[] =3D {
+ { .compatible =3D "skyworks,sky81452", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sky81452_of_match);
+#endif
+
+static struct i2c_driver sky81452_driver =3D {
+ .driver =3D {
+ .name =3D "sky81452",
+ .of_match_table =3D of_match_ptr(sky81452_of_match),
+ },
+ .probe =3D sky81452_probe,
+ .remove =3D sky81452_remove,
+ .id_table =3D sky81452_ids,
+};
+
+module_i2c_driver(sky81452_driver);
+
+MODULE_DESCRIPTION("Skyworks SKY81452 MFD driver");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("1.2");
diff --git a/include/linux/mfd/sky81452.h b/include/linux/mfd/sky8145=

2.h

Post by Gyungoh Yoo
new file mode 100644
index 0000000..142f762
--- /dev/null
+++ b/include/linux/mfd/sky81452.h
@@ -0,0 +1,32 @@
+/*
+ * sky81452.h SKY81452 MFD driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ *
+ * This program is free software; you can redistribute it and/or mod=

ify it

Post by Gyungoh Yoo
+ * under the terms of the GNU General Public License as published by=

the

Post by Gyungoh Yoo
+ * Free Software Foundation; either version 2, or (at your option) a=

Post by Gyungoh Yoo
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, b=

along

Post by Gyungoh Yoo
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _SKY81452_H
+#define _SKY81452_H
+
+#include <linux/sky81452-backlight.h>
+#include <linux/regulator/machine.h>
+
+struct sky81452_platform_data {
+ struct sky81452_bl_platform_data *bl_pdata;
+ struct regulator_init_data *regulator_init_data;
+};
+
+#endif

--=20
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org =E2=94=82 Open source software for ARM SoCs
=46ollow Linaro: Facebook | Twitter | Blog

Gyungoh Yoo

2014-10-07 08:23:23 UTC

Post by Lee Jones
You say that this is v3, but there is no change log (usually placed
between the '---' and the diffstat). Also you have 8 patches in this
set with no 0/8 patch explaining what you're trying to achieve. For
this size patch you also require a patch commit log. What does this
driver provide? How does it provide it? Once you've addressed these
issues, I'll review your patches.

I sent 0/8 patch, and it has the change log and what this patch is
trying to do.
Can I ask you to review the patches?
Thank you.

Post by Gyungoh Yoo
---
drivers/mfd/Kconfig | 12 +++++
drivers/mfd/Makefile | 1 +
drivers/mfd/sky81452.c | 111 +++++++++++++++++++++++++++++++=

++++++++++++

=2Eo

Post by Gyungoh Yoo
obj-$(CONFIG_INTEL_SOC_PMIC) +=3D intel-soc-pmic.o
diff --git a/drivers/mfd/sky81452.c b/drivers/mfd/sky81452.c
new file mode 100644
index 0000000..9130112
--- /dev/null
+++ b/drivers/mfd/sky81452.c
@@ -0,0 +1,111 @@
+/*
+ * sky81452.c SKY81452 MFD driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ *
+ * This program is free software; you can redistribute it and/or m=

odify it

Post by Gyungoh Yoo
+ * under the terms of the GNU General Public License as published =

by the

Post by Gyungoh Yoo
+ * Free Software Foundation; either version 2, or (at your option)=

any

Post by Gyungoh Yoo
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful,=

but

Post by Gyungoh Yoo
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the G=

Post by Gyungoh Yoo
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public Licen=

se along

ev);

;

Post by Gyungoh Yoo
+ return PTR_ERR(regmap);
+ }
+
+ i2c_set_clientdata(client, regmap);
+
+ memset(cells, 0, sizeof(cells));
+ cells[0].name =3D "sky81452-backlight";
+ cells[0].of_compatible =3D "skyworks,sky81452-backlight";
+ cells[0].platform_data =3D pdata->bl_pdata;
+ cells[0].pdata_size =3D sizeof(*pdata->bl_pdata);
+ cells[1].name =3D "sky81452-regulator";
+ cells[1].of_compatible =3D "skyworks,sky81452-regulator";
+ cells[1].platform_data =3D pdata->regulator_init_data;
+ cells[1].pdata_size =3D sizeof(*pdata->regulator_init_data);
+
+ ret =3D mfd_add_devices(dev, -1, cells, ARRAY_SIZE(cells), NULL, =

0, NULL);

452.h

odify it

Post by Gyungoh Yoo
+ * under the terms of the GNU General Public License as published =

by the

Post by Gyungoh Yoo
+ * Free Software Foundation; either version 2, or (at your option)=

any

Post by Gyungoh Yoo
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful,=

but

Post by Gyungoh Yoo
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the G=

Post by Gyungoh Yoo
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public Licen=

se along

=20
--=20
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org =E2=94=82 Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

--
To unsubscribe from this list: send the line "unsubscribe devicetree" i=
n
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Gyungoh Yoo

2014-10-07 05:01:15 UTC

Signed-off-by: Gyungoh Yoo <jack.yoo-***@public.gmane.org>
---
drivers/video/backlight/Kconfig | 10 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/sky81452-backlight.c | 347 +++++++++++++++++++++++++++
3 files changed, 358 insertions(+)
create mode 100644 drivers/video/backlight/sky81452-backlight.c

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 8d03924..2586fdd 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -409,6 +409,16 @@ config BACKLIGHT_PANDORA
If you have a Pandora console, say Y to enable the
backlight driver.

+config BACKLIGHT_SKY81452
+ tristate "Backlight driver for SKY81452"
+ depends on BACKLIGHT_CLASS_DEVICE && MFD_SKY81452
+ help
+ If you have a Skyworks SKY81452, say Y to enable the
+ backlight driver.
+
+ To compile this driver as a module, choose M here: the module will
+ be called sky81452-backlight
+
config BACKLIGHT_TPS65217
tristate "TPS65217 Backlight"
depends on BACKLIGHT_CLASS_DEVICE && MFD_TPS65217
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index fcd50b73..d67073f 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_BACKLIGHT_PANDORA) += pandora_bl.o
obj-$(CONFIG_BACKLIGHT_PCF50633) += pcf50633-backlight.o
obj-$(CONFIG_BACKLIGHT_PWM) += pwm_bl.o
obj-$(CONFIG_BACKLIGHT_SAHARA) += kb3886_bl.o
+obj-$(CONFIG_BACKLIGHT_SKY81452) += sky81452-backlight.o
obj-$(CONFIG_BACKLIGHT_TOSA) += tosa_bl.o
obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o
obj-$(CONFIG_BACKLIGHT_WM831X) += wm831x_bl.o
diff --git a/drivers/video/backlight/sky81452-backlight.c b/drivers/video/backlight/sky81452-backlight.c
new file mode 100644
index 0000000..101399d
--- /dev/null
+++ b/drivers/video/backlight/sky81452-backlight.c
@@ -0,0 +1,347 @@
+/*
+ * sky81452-backlight.c SKY81452 backlight driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo <jack.yoo-***@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/regmap.h>
+#include <linux/backlight.h>
+#include <linux/sky81452-backlight.h>
+
+/* registers */
+#define SKY81452_REG0 0x00
+#define SKY81452_REG1 0x01
+#define SKY81452_REG2 0x02
+#define SKY81452_REG4 0x04
+#define SKY81452_REG5 0x05
+
+/* bit mask */
+#define SKY81452_CS 0xFF
+#define SKY81452_EN 0x3F
+#define SKY81452_IGPW 0x20
+#define SKY81452_PWMMD 0x10
+#define SKY81452_PHASE 0x08
+#define SKY81452_ILIM 0x04
+#define SKY81452_VSHRT 0x03
+#define SKY81452_OCP 0x80
+#define SKY81452_OTMP 0x40
+#define SKY81452_SHRT 0x3F
+#define SKY81452_OPN 0x3F
+
+#define SKY81452_DEFAULT_NAME "lcd-backlight"
+#define SKY81452_MAX_BRIGHTNESS (SKY81452_CS + 1)
+
+#define CTZ(b) __builtin_ctz(b)
+
+static int sky81452_bl_update_status(struct backlight_device *bd)
+{
+ const struct sky81452_bl_platform_data *pdata =
+ dev_get_platdata(bd->dev.parent);
+ const unsigned int brightness = (unsigned int)bd->props.brightness;
+ struct regmap *regmap = bl_get_data(bd);
+ int ret;
+
+ if (brightness > 0) {
+ ret = regmap_write(regmap, SKY81452_REG0, brightness - 1);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ return regmap_update_bits(regmap, SKY81452_REG1, SKY81452_EN,
+ pdata->enable << CTZ(SKY81452_EN));
+ }
+
+ return regmap_update_bits(regmap, SKY81452_REG1, SKY81452_EN, 0);
+}
+
+static int sky81452_bl_get_brightness(struct backlight_device *bd)
+{
+ return bd->props.brightness;
+}
+
+static const struct backlight_ops sky81452_bl_ops = {
+ .update_status = sky81452_bl_update_status,
+ .get_brightness = sky81452_bl_get_brightness,
+};
+
+static ssize_t sky81452_bl_store_enable(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct regmap *regmap = bl_get_data(to_backlight_device(dev));
+ unsigned long value;
+ int ret;
+
+ ret = kstrtoul(buf, 16, &value);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ ret = regmap_update_bits(regmap, SKY81452_REG1, SKY81452_EN,
+ value << CTZ(SKY81452_EN));
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ return count;
+}
+
+static ssize_t sky81452_bl_show_open_short(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct regmap *regmap = bl_get_data(to_backlight_device(dev));
+ unsigned int reg, value = 0;
+ char tmp[3];
+ int i, ret;
+
+ reg = !strcmp(attr->attr.name, "open") ? SKY81452_REG5 : SKY81452_REG4;
+ ret = regmap_read(regmap, reg, &value);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ if (value & SKY81452_SHRT) {
+ *buf = 0;
+ for (i = 0; i < 6; i++) {
+ if (value & 0x01) {
+ sprintf(tmp, "%d ", i + 1);
+ strcat(buf, tmp);
+ }
+ value >>= 1;
+ }
+ strcat(buf, "\n");
+ } else
+ strcpy(buf, "none\n");
+
+ return strlen(buf);
+}
+
+static ssize_t sky81452_bl_show_fault(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct regmap *regmap = bl_get_data(to_backlight_device(dev));
+ unsigned int value = 0;
+ int ret;
+
+ ret = regmap_read(regmap, SKY81452_REG4, &value);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ *buf = 0;
+
+ if (value & SKY81452_OCP)
+ strcat(buf, "over-current ");
+
+ if (value & SKY81452_OTMP)
+ strcat(buf, "over-temperature");
+
+ strcat(buf, "\n");
+ return strlen(buf);
+}
+
+static DEVICE_ATTR(enable, S_IWGRP | S_IWUSR, NULL, sky81452_bl_store_enable);
+static DEVICE_ATTR(open, S_IRUGO, sky81452_bl_show_open_short, NULL);
+static DEVICE_ATTR(short, S_IRUGO, sky81452_bl_show_open_short, NULL);
+static DEVICE_ATTR(fault, S_IRUGO, sky81452_bl_show_fault, NULL);
+
+static struct attribute *sky81452_bl_attribute[] = {
+ &dev_attr_enable.attr,
+ &dev_attr_open.attr,
+ &dev_attr_short.attr,
+ &dev_attr_fault.attr,
+ NULL
+};
+
+static const struct attribute_group sky81452_bl_attr_group = {
+ .attrs = sky81452_bl_attribute,
+};
+
+#ifdef CONFIG_OF
+static struct sky81452_bl_platform_data *sky81452_bl_parse_dt(
+ struct device *dev)
+{
+ struct device_node *np = of_node_get(dev->of_node);
+ struct sky81452_bl_platform_data *pdata;
+ int ret;
+
+ if (!np) {
+ dev_err(dev, "backlight node not found");
+ return ERR_PTR(-ENODATA);
+ }
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata) {
+ of_node_put(np);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ of_property_read_string(np, "name", &pdata->name);
+ pdata->ignore_pwm = of_property_read_bool(np, "ignore-pwm");
+ pdata->dpwm_mode = of_property_read_bool(np, "dpwm-mode");
+ pdata->phase_shift = of_property_read_bool(np, "phase-shift");
+
+ pdata->gpio_enable = of_get_named_gpio(np, "gpio-enable", 0);
+ if (IS_ERR_VALUE(pdata->gpio_enable))
+ pdata->gpio_enable = -1;
+
+ ret = of_property_read_u32(np, "enable", &pdata->enable);
+ if (IS_ERR_VALUE(ret))
+ pdata->enable = SKY81452_EN >> CTZ(SKY81452_EN);
+
+ ret = of_property_read_u32(np, "short-detection-threshold",
+ &pdata->short_detection_threshold);
+ if (IS_ERR_VALUE(ret))
+ pdata->short_detection_threshold = 7;
+
+ ret = of_property_read_u32(np, "boost-current-limit",
+ &pdata->boost_current_limit);
+ if (IS_ERR_VALUE(ret))
+ pdata->boost_current_limit = 2750;
+
+ of_node_put(np);
+ return pdata;
+}
+#else
+static struct sky81452_bl_platform_data *sky81452_bl_parse_dt(
+ struct device *dev)
+{
+ return ERR_PTR(-EINVAL);
+}
+#endif
+
+static int sky81452_bl_init_device(struct regmap *regmap,
+ struct sky81452_bl_platform_data *pdata)
+{
+ unsigned int value;
+
+ value = pdata->ignore_pwm ? SKY81452_IGPW : 0;
+ value |= pdata->dpwm_mode ? SKY81452_PWMMD : 0;
+ value |= pdata->phase_shift ? 0 : SKY81452_PHASE;
+
+ if (pdata->boost_current_limit == 2300)
+ value |= SKY81452_ILIM;
+ else if (pdata->boost_current_limit != 2720)
+ return -EINVAL;
+
+ if (pdata->short_detection_threshold < 4 ||
+ pdata->short_detection_threshold > 7)
+ return -EINVAL;
+ value |= (7 - pdata->short_detection_threshold) << CTZ(SKY81452_VSHRT);
+
+ return regmap_write(regmap, SKY81452_REG2, value);
+}
+
+static int sky81452_bl_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct regmap *regmap = dev_get_drvdata(dev->parent);
+ struct sky81452_bl_platform_data *pdata = dev_get_platdata(dev);
+ struct backlight_device *bd;
+ struct backlight_properties props;
+ const char *name;
+ int ret;
+
+ if (!pdata) {
+ pdata = sky81452_bl_parse_dt(dev);
+ if (IS_ERR(pdata))
+ return PTR_ERR(pdata);
+ }
+
+ if (pdata->gpio_enable >= 0) {
+ ret = devm_gpio_request_one(dev, pdata->gpio_enable,
+ GPIOF_OUT_INIT_HIGH, "sky81452-en");
+ if (IS_ERR_VALUE(ret)) {
+ dev_err(dev, "failed to request GPIO. err=%d", ret);
+ return ret;
+ }
+ }
+
+ ret = sky81452_bl_init_device(regmap, pdata);
+ if (IS_ERR_VALUE(ret)) {
+ dev_err(dev, "failed to initialize. err=%d", ret);
+ return ret;
+ }
+
+ memset(&props, 0, sizeof(props));
+ props.max_brightness = SKY81452_MAX_BRIGHTNESS,
+ name = pdata->name ? pdata->name : SKY81452_DEFAULT_NAME;
+ bd = devm_backlight_device_register(dev, name, dev, regmap,
+ &sky81452_bl_ops, &props);
+ if (IS_ERR(bd)) {
+ dev_err(dev, "failed to register. err=%ld", PTR_ERR(bd));
+ return PTR_ERR(bd);
+ }
+
+ platform_set_drvdata(pdev, bd);
+
+ ret = sysfs_create_group(&bd->dev.kobj, &sky81452_bl_attr_group);
+ if (IS_ERR_VALUE(ret)) {
+ dev_err(dev, "failed to create attribute. err=%d", ret);
+ goto err;
+ }
+
+ return ret;
+err:
+ backlight_device_unregister(bd);
+ return ret;
+}
+
+static int sky81452_bl_remove(struct platform_device *pdev)
+{
+ const struct sky81452_bl_platform_data *pdata =
+ dev_get_platdata(&pdev->dev);
+ struct backlight_device *bd = platform_get_drvdata(pdev);
+
+ sysfs_remove_group(&bd->dev.kobj, &sky81452_bl_attr_group);
+
+ bd->props.power = FB_BLANK_UNBLANK;
+ bd->props.brightness = 0;
+ backlight_update_status(bd);
+
+ if (pdata->gpio_enable >= 0)
+ gpio_set_value_cansleep(pdata->gpio_enable, 0);
+
+ return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id sky81452_bl_of_match[] = {
+ { .compatible = "skyworks,sky81452-backlight", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sky81452_bl_of_match);
+#endif
+
+static struct platform_driver sky81452_bl_driver = {
+ .driver = {
+ .name = "sky81452-backlight",
+ .of_match_table = of_match_ptr(sky81452_bl_of_match),
+ },
+ .probe = sky81452_bl_probe,
+ .remove = sky81452_bl_remove,
+};
+
+module_platform_driver(sky81452_bl_driver);
+
+MODULE_DESCRIPTION("Skyworks SKY81452 backlight driver");
+MODULE_AUTHOR("Gyungoh Yoo <jack.yoo-***@public.gmane.org>");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("1.2");

Jingoo Han

2014-10-08 01:03:57 UTC

On Tuesday, October 07, 2014 2:01 PM, Gyungoh Yoo wrote:
If possible, please add more detailed commit message for this patch.

Post by Gyungoh Yoo
---
drivers/video/backlight/Kconfig | 10 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/sky81452-backlight.c | 347 +++++++++++++++++++++++++++
3 files changed, 358 insertions(+)
create mode 100644 drivers/video/backlight/sky81452-backlight.c
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 8d03924..2586fdd 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -409,6 +409,16 @@ config BACKLIGHT_PANDORA
If you have a Pandora console, say Y to enable the
backlight driver.
+config BACKLIGHT_SKY81452
+ tristate "Backlight driver for SKY81452"
+ depends on BACKLIGHT_CLASS_DEVICE && MFD_SKY81452
+ help
+ If you have a Skyworks SKY81452, say Y to enable the
+ backlight driver.
+
+ To compile this driver as a module, choose M here: the module will
+ be called sky81452-backlight
+
config BACKLIGHT_TPS65217
tristate "TPS65217 Backlight"
depends on BACKLIGHT_CLASS_DEVICE && MFD_TPS65217
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index fcd50b73..d67073f 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_BACKLIGHT_PANDORA) += pandora_bl.o
obj-$(CONFIG_BACKLIGHT_PCF50633) += pcf50633-backlight.o
obj-$(CONFIG_BACKLIGHT_PWM) += pwm_bl.o
obj-$(CONFIG_BACKLIGHT_SAHARA) += kb3886_bl.o
+obj-$(CONFIG_BACKLIGHT_SKY81452) += sky81452-backlight.o
obj-$(CONFIG_BACKLIGHT_TOSA) += tosa_bl.o
obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o
obj-$(CONFIG_BACKLIGHT_WM831X) += wm831x_bl.o
diff --git a/drivers/video/backlight/sky81452-backlight.c b/drivers/video/backlight/sky81452-
backlight.c
new file mode 100644
index 0000000..101399d
--- /dev/null
+++ b/drivers/video/backlight/sky81452-backlight.c
@@ -0,0 +1,347 @@
+/*
+ * sky81452-backlight.c SKY81452 backlight driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/regmap.h>
+#include <linux/backlight.h>
+#include <linux/sky81452-backlight.h>

Please, re-order these headers alphabetically.
It enhances the readability.

Post by Gyungoh Yoo
+
+/* registers */
+#define SKY81452_REG0 0x00
+#define SKY81452_REG1 0x01
+#define SKY81452_REG2 0x02
+#define SKY81452_REG4 0x04
+#define SKY81452_REG5 0x05
+
+/* bit mask */
+#define SKY81452_CS 0xFF
+#define SKY81452_EN 0x3F
+#define SKY81452_IGPW 0x20
+#define SKY81452_PWMMD 0x10
+#define SKY81452_PHASE 0x08
+#define SKY81452_ILIM 0x04
+#define SKY81452_VSHRT 0x03
+#define SKY81452_OCP 0x80
+#define SKY81452_OTMP 0x40
+#define SKY81452_SHRT 0x3F
+#define SKY81452_OPN 0x3F
+
+#define SKY81452_DEFAULT_NAME "lcd-backlight"
+#define SKY81452_MAX_BRIGHTNESS (SKY81452_CS + 1)
+
+#define CTZ(b) __builtin_ctz(b)
+
+static int sky81452_bl_update_status(struct backlight_device *bd)
+{
+ const struct sky81452_bl_platform_data *pdata =
+ dev_get_platdata(bd->dev.parent);
+ const unsigned int brightness = (unsigned int)bd->props.brightness;
+ struct regmap *regmap = bl_get_data(bd);
+ int ret;
+
+ if (brightness > 0) {
+ ret = regmap_write(regmap, SKY81452_REG0, brightness - 1);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ return regmap_update_bits(regmap, SKY81452_REG1, SKY81452_EN,
+ pdata->enable << CTZ(SKY81452_EN));
+ }
+
+ return regmap_update_bits(regmap, SKY81452_REG1, SKY81452_EN, 0);
+}
+
+static int sky81452_bl_get_brightness(struct backlight_device *bd)
+{
+ return bd->props.brightness;
+}
+
+static const struct backlight_ops sky81452_bl_ops = {
+ .update_status = sky81452_bl_update_status,
+ .get_brightness = sky81452_bl_get_brightness,
+};
+
+static ssize_t sky81452_bl_store_enable(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct regmap *regmap = bl_get_data(to_backlight_device(dev));
+ unsigned long value;
+ int ret;
+
+ ret = kstrtoul(buf, 16, &value);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ ret = regmap_update_bits(regmap, SKY81452_REG1, SKY81452_EN,
+ value << CTZ(SKY81452_EN));
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ return count;
+}
+
+static ssize_t sky81452_bl_show_open_short(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct regmap *regmap = bl_get_data(to_backlight_device(dev));
+ unsigned int reg, value = 0;
+ char tmp[3];
+ int i, ret;
+
+ reg = !strcmp(attr->attr.name, "open") ? SKY81452_REG5 : SKY81452_REG4;
+ ret = regmap_read(regmap, reg, &value);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ if (value & SKY81452_SHRT) {
+ *buf = 0;
+ for (i = 0; i < 6; i++) {
+ if (value & 0x01) {
+ sprintf(tmp, "%d ", i + 1);
+ strcat(buf, tmp);
+ }
+ value >>= 1;
+ }
+ strcat(buf, "\n");
+ } else
+ strcpy(buf, "none\n");

Please add braces to 'else' statement.

If only one branch of a conditional statement is a single
statement. Braces are needed.

if () {
... (multi lines)
} else {
...
}

Post by Gyungoh Yoo
+
+ return strlen(buf);
+}
+
+static ssize_t sky81452_bl_show_fault(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct regmap *regmap = bl_get_data(to_backlight_device(dev));
+ unsigned int value = 0;
+ int ret;
+
+ ret = regmap_read(regmap, SKY81452_REG4, &value);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ *buf = 0;
+
+ if (value & SKY81452_OCP)
+ strcat(buf, "over-current ");
+
+ if (value & SKY81452_OTMP)
+ strcat(buf, "over-temperature");
+
+ strcat(buf, "\n");
+ return strlen(buf);
+}
+
+static DEVICE_ATTR(enable, S_IWGRP | S_IWUSR, NULL, sky81452_bl_store_enable);
+static DEVICE_ATTR(open, S_IRUGO, sky81452_bl_show_open_short, NULL);
+static DEVICE_ATTR(short, S_IRUGO, sky81452_bl_show_open_short, NULL);
+static DEVICE_ATTR(fault, S_IRUGO, sky81452_bl_show_fault, NULL);
+
+static struct attribute *sky81452_bl_attribute[] = {
+ &dev_attr_enable.attr,
+ &dev_attr_open.attr,
+ &dev_attr_short.attr,
+ &dev_attr_fault.attr,
+ NULL
+};
+
+static const struct attribute_group sky81452_bl_attr_group = {
+ .attrs = sky81452_bl_attribute,
+};
+
+#ifdef CONFIG_OF
+static struct sky81452_bl_platform_data *sky81452_bl_parse_dt(
+ struct device *dev)
+{
+ struct device_node *np = of_node_get(dev->of_node);
+ struct sky81452_bl_platform_data *pdata;
+ int ret;
+
+ if (!np) {
+ dev_err(dev, "backlight node not found");
+ return ERR_PTR(-ENODATA);
+ }
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata) {
+ of_node_put(np);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ of_property_read_string(np, "name", &pdata->name);
+ pdata->ignore_pwm = of_property_read_bool(np, "ignore-pwm");
+ pdata->dpwm_mode = of_property_read_bool(np, "dpwm-mode");
+ pdata->phase_shift = of_property_read_bool(np, "phase-shift");
+
+ pdata->gpio_enable = of_get_named_gpio(np, "gpio-enable", 0);
+ if (IS_ERR_VALUE(pdata->gpio_enable))
+ pdata->gpio_enable = -1;
+
+ ret = of_property_read_u32(np, "enable", &pdata->enable);
+ if (IS_ERR_VALUE(ret))
+ pdata->enable = SKY81452_EN >> CTZ(SKY81452_EN);
+
+ ret = of_property_read_u32(np, "short-detection-threshold",
+ &pdata->short_detection_threshold);
+ if (IS_ERR_VALUE(ret))
+ pdata->short_detection_threshold = 7;
+
+ ret = of_property_read_u32(np, "boost-current-limit",
+ &pdata->boost_current_limit);
+ if (IS_ERR_VALUE(ret))
+ pdata->boost_current_limit = 2750;
+
+ of_node_put(np);
+ return pdata;
+}
+#else
+static struct sky81452_bl_platform_data *sky81452_bl_parse_dt(
+ struct device *dev)
+{
+ return ERR_PTR(-EINVAL);
+}
+#endif
+
+static int sky81452_bl_init_device(struct regmap *regmap,
+ struct sky81452_bl_platform_data *pdata)
+{
+ unsigned int value;
+
+ value = pdata->ignore_pwm ? SKY81452_IGPW : 0;
+ value |= pdata->dpwm_mode ? SKY81452_PWMMD : 0;
+ value |= pdata->phase_shift ? 0 : SKY81452_PHASE;
+
+ if (pdata->boost_current_limit == 2300)
+ value |= SKY81452_ILIM;
+ else if (pdata->boost_current_limit != 2720)
+ return -EINVAL;
+
+ if (pdata->short_detection_threshold < 4 ||
+ pdata->short_detection_threshold > 7)
+ return -EINVAL;
+ value |= (7 - pdata->short_detection_threshold) << CTZ(SKY81452_VSHRT);
+
+ return regmap_write(regmap, SKY81452_REG2, value);
+}
+
+static int sky81452_bl_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct regmap *regmap = dev_get_drvdata(dev->parent);
+ struct sky81452_bl_platform_data *pdata = dev_get_platdata(dev);
+ struct backlight_device *bd;
+ struct backlight_properties props;
+ const char *name;
+ int ret;
+
+ if (!pdata) {
+ pdata = sky81452_bl_parse_dt(dev);
+ if (IS_ERR(pdata))
+ return PTR_ERR(pdata);
+ }
+
+ if (pdata->gpio_enable >= 0) {
+ ret = devm_gpio_request_one(dev, pdata->gpio_enable,
+ GPIOF_OUT_INIT_HIGH, "sky81452-en");
+ if (IS_ERR_VALUE(ret)) {
+ dev_err(dev, "failed to request GPIO. err=%d", ret);
+ return ret;
+ }
+ }
+
+ ret = sky81452_bl_init_device(regmap, pdata);
+ if (IS_ERR_VALUE(ret)) {
+ dev_err(dev, "failed to initialize. err=%d", ret);
+ return ret;
+ }
+
+ memset(&props, 0, sizeof(props));
+ props.max_brightness = SKY81452_MAX_BRIGHTNESS,
+ name = pdata->name ? pdata->name : SKY81452_DEFAULT_NAME;
+ bd = devm_backlight_device_register(dev, name, dev, regmap,
+ &sky81452_bl_ops, &props);
+ if (IS_ERR(bd)) {
+ dev_err(dev, "failed to register. err=%ld", PTR_ERR(bd));
+ return PTR_ERR(bd);
+ }
+
+ platform_set_drvdata(pdev, bd);
+
+ ret = sysfs_create_group(&bd->dev.kobj, &sky81452_bl_attr_group);
+ if (IS_ERR_VALUE(ret)) {
+ dev_err(dev, "failed to create attribute. err=%d", ret);
+ goto err;
+ }
+
+ return ret;
+ backlight_device_unregister(bd);

This backlight_device_unregister() is NOT necessary.
This is because backlight_device_unregister will be called
automatically, when devm_backlight_device_register() is used
and error returns.

Post by Gyungoh Yoo
+ return ret;
+}
+
+static int sky81452_bl_remove(struct platform_device *pdev)
+{
+ const struct sky81452_bl_platform_data *pdata =
+ dev_get_platdata(&pdev->dev);
+ struct backlight_device *bd = platform_get_drvdata(pdev);
+
+ sysfs_remove_group(&bd->dev.kobj, &sky81452_bl_attr_group);
+
+ bd->props.power = FB_BLANK_UNBLANK;
+ bd->props.brightness = 0;
+ backlight_update_status(bd);
+
+ if (pdata->gpio_enable >= 0)
+ gpio_set_value_cansleep(pdata->gpio_enable, 0);
+
+ return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id sky81452_bl_of_match[] = {
+ { .compatible = "skyworks,sky81452-backlight", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sky81452_bl_of_match);
+#endif
+
+static struct platform_driver sky81452_bl_driver = {
+ .driver = {
+ .name = "sky81452-backlight",
+ .of_match_table = of_match_ptr(sky81452_bl_of_match),
+ },
+ .probe = sky81452_bl_probe,
+ .remove = sky81452_bl_remove,
+};
+
+module_platform_driver(sky81452_bl_driver);
+
+MODULE_DESCRIPTION("Skyworks SKY81452 backlight driver");
+MODULE_LICENSE("GPL");

How about using 'GPL v2'?

Post by Gyungoh Yoo
+MODULE_VERSION("1.2");
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Gyungoh Yoo

2014-10-08 08:55:34 UTC

Post by Jingoo Han
If possible, please add more detailed commit message for this patch.

Please, re-order these headers alphabetically.
It enhances the readability.

I see.

Please add braces to 'else' statement.
If only one branch of a conditional statement is a single
statement. Braces are needed.
if () {
... (multi lines)
} else {
...
}

I see.

This backlight_device_unregister() is NOT necessary.
This is because backlight_device_unregister will be called
automatically, when devm_backlight_device_register() is used
and error returns.

I see.

How about using 'GPL v2'?

I will change it.
Thanks you so much for your review.

Post by Gyungoh Yoo
+MODULE_VERSION("1.2");
--
1.9.1

Gyungoh Yoo

2014-10-07 05:04:11 UTC

Signed-off-by: Gyungoh Yoo <***@skyworksinc.com>
---
Documentation/devicetree/bindings/mfd/sky81452.txt | 32 ++++++++++++++++++++++
1 file changed, 32 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/sky81452.txt

diff --git a/Documentation/devicetree/bindings/mfd/sky81452.txt b/Documentation/devicetree/bindings/mfd/sky81452.txt
new file mode 100644
index 0000000..d61904a
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/sky81452.txt
@@ -0,0 +1,32 @@
+SKY81452 bindings
+
+Required properties:
+- compatible : Must be "skyworks,sky81452"
+- reg : I2C slave address
+
+Required child nodes:
+- backlight : container node for backlight following the binding
+ in video/backlight/sky81452-backlight.txt
+- regulator : container node for regulators following the binding
+ in regulator/sky81452-regulator.txt
+
+Example:
+
+ ***@2c {
+ compatible = "skyworks,sky81452";
+ reg = <0x2c>;
+
+ backlight {
+ compatible = "skyworks,sky81452-backlight";
+ name = "pwm-backlight";
+ enable = <0x3F>;
+ ignore-pwm;
+ };
+
+ regulator {
+ compatible = "skyworks,sky81452-regulator";
+ regulator-name = "touch_en";
+ regulator-min-microvolt = <4500000>;
+ regulator-max-microvolt = <8000000>;
+ };
+ };

--
1.9.1

Gyungoh Yoo

2014-10-07 05:04:56 UTC

Signed-off-by: Gyungoh Yoo <jack.yoo-***@public.gmane.org>
---
.../video/backlight/sky81452-backlight.txt | 24 ++++++++++++++++++++++
1 file changed, 24 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt

diff --git a/Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt b/Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt
new file mode 100644
index 0000000..2c2d947
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt
@@ -0,0 +1,24 @@
+SKY81452-backlight bindings
+
+Required properties:
+- compatible : Must be "skyworks,sky81452-backlight"
+
+Optional properties:
+- name : Name of backlight device. Default is 'lcd-backlight'.
+- gpio-enable : GPIO to use to EN pin.
+- enable : Enable mask for current sink channel 1 to 6.
+- ignore-pwm : Ignore both PWM input
+- dpwm-mode : Enable DPWM dimming mode, otherwise Analog dimming mode
+- phase-shift : Enable phase shift mode
+- ovp-level : Over-voltage protection level. Should be between 14 or 28V.
+- short-detection-threshold : It should be one of 4, 5, 6 and 7V.
+- boost-current-limit : It should be one of 800, 1100 and 1500mA.
+
+Example:
+
+ backlight {
+ compatible = "skyworks,sky81452-backlight";
+ name = "pwm-backlight";
+ enable = <0x3F>;
+ ignore-pwm;
+ };

Gyungoh Yoo

2014-10-07 05:06:56 UTC

Signed-off-by: Gyungoh Yoo <jack.yoo-***@public.gmane.org>
---
Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index ac7269f..c755978 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -125,6 +125,7 @@ silabs Silicon Laboratories
simtek
sii Seiko Instruments, Inc.
sirf SiRF Technology, Inc.
+skyworks Skyworks Solutions, Inc.
smsc Standard Microsystems Corporation
snps Synopsys, Inc.
solidrun SolidRun

Gyungoh Yoo

2014-10-07 05:09:20 UTC

Signed-off-by: Gyungoh Yoo <jack.yoo-***@public.gmane.org>
---
Documentation/devicetree/bindings/i2c/trivial-devices.txt | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index 6af570e..ff77879 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -78,6 +78,7 @@ ramtron,24c64 i2c serial eeprom (24cxx)
ricoh,rs5c372a I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
samsung,24ad0xd1 S524AD0XF1 (128K/256K-bit Serial EEPROM for Low Power)
sii,s35390a 2-wire CMOS real-time clock
+skyworks,sky81452 Skyworks SKY81452: Six-Channel White LED Driver with Touch Panel Bias Supply
st-micro,24c256 i2c serial eeprom (24cxx)
stm,m41t00 Serial Access TIMEKEEPER
stm,m41t62 Serial real-time clock (RTC) with alarm

Gyungoh Yoo

2014-10-07 05:11:07 UTC

Signed-off-by: Gyungoh Yoo <jack.yoo-***@public.gmane.org>
---
drivers/regulator/sky81452-regulator.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/regulator/sky81452-regulator.c b/drivers/regulator/sky81452-regulator.c
index 97aff0c..f895d1b 100644
--- a/drivers/regulator/sky81452-regulator.c
+++ b/drivers/regulator/sky81452-regulator.c
@@ -68,12 +68,11 @@ static const struct regulator_desc sky81452_reg = {
static struct regulator_init_data *sky81452_reg_parse_dt(struct device *dev)
{
struct regulator_init_data *init_data;
- struct device_node *np;
+ struct device_node *np = of_node_get(dev->of_node);

- np = of_get_child_by_name(dev->parent->of_node, "regulator");
- if (unlikely(!np)) {
+ if (!np) {
dev_err(dev, "regulator node not found");
- return NULL;
+ return ERR_PTR(-ENODATA);
}

init_data = of_get_regulator_init_data(dev, np);
@@ -107,17 +106,28 @@ static int sky81452_reg_probe(struct platform_device *pdev)
config.regmap = dev_get_drvdata(dev->parent);

rdev = devm_regulator_register(dev, &sky81452_reg, &config);
- if (IS_ERR(rdev))
+ if (IS_ERR(rdev)) {
+ dev_err(dev, "failed to register. err=%ld", PTR_ERR(rdev));
return PTR_ERR(rdev);
+ }

platform_set_drvdata(pdev, rdev);

return 0;
}

+#ifdef CONFIG_OF
+static const struct of_device_id sky81452_reg_of_match[] = {
+ { .compatible = "skyworks,sky81452-regulator", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sky81452_reg_of_match);
+#endif
+
static struct platform_driver sky81452_reg_driver = {
.driver = {
.name = "sky81452-regulator",
+ .of_match_table = of_match_ptr(sky81452_reg_of_match),
},
.probe = sky81452_reg_probe,
};
@@ -127,4 +137,4 @@ module_platform_driver(sky81452_reg_driver);
MODULE_DESCRIPTION("Skyworks SKY81452 Regulator driver");
MODULE_AUTHOR("Gyungoh Yoo <jack.yoo-***@public.gmane.org>");
MODULE_LICENSE("GPL");
-MODULE_VERSION("1.0");
+MODULE_VERSION("1.2");

Mark Brown

2014-10-07 12:52:21 UTC

Several problems here:

- I don't have patches 1-6 or the cover letter for this series - what
are the dependencies?
- I can't see any sign that my concerns about the driver not actually
being reusable have been addressed here.
- This patch appears to do way more than add a compatible string, for

Post by Gyungoh Yoo
- np = of_get_child_by_name(dev->parent->of_node, "regulator");
- if (unlikely(!np)) {
+ if (!np) {
dev_err(dev, "regulator node not found");
- return NULL;
+ return ERR_PTR(-ENODATA);

Gyungoh Yoo

2014-10-08 08:51:29 UTC

Post by Mark Brown
- I don't have patches 1-6 or the cover letter for this series - what
are the dependencies?
- I can't see any sign that my concerns about the driver not actually
being reusable have been addressed here.
- This patch appears to do way more than add a compatible string, for

I am sorry the recipients of each patch and cover letter was different.
I will resubmit soon.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Gyungoh Yoo

2014-10-07 05:13:11 UTC

Signed-off-by: Gyungoh Yoo <jack.yoo-***@public.gmane.org>
---
Documentation/devicetree/bindings/regulator/sky81452-regulator.txt | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/sky81452-regulator.txt b/Documentation/devicetree/bindings/regulator/sky81452-regulator.txt
index 882455e..7b9ff18 100644
--- a/Documentation/devicetree/bindings/regulator/sky81452-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/sky81452-regulator.txt
@@ -1,6 +1,7 @@
SKY81452 voltage regulator

Required properties:
+- compatible : Must be "skyworks,sky81452-regulator"
- any required generic properties defined in regulator.txt

Optional properties:
@@ -9,6 +10,7 @@ Optional properties:
Example:

regulator {
+ compatible = "skyworks,sky81452-regulator";
/* generic regulator properties */
regulator-name = "touch_en";
regulator-min-microvolt = <4500000>;

Gyungoh Yoo

2014-10-13 05:47:34 UTC

This patch set includes regulator and backlight driver for SKY81452.
Also it includes documents for device tree and module.
The initial version of sky81452-regulator was applied. Fo this, incremental
patches are included.

v4:
Removed MODULE_VERSION()
Modified license to GPLv2
Removed calling to backlight_device_unregister() in sky81452-backlight

v3:
Cleaned-up DBG messages
Cleaned-up DT
Fixed the backlight name from 'sky81452-bl' to 'sky81452-backlight'
Assigned mfd_cell.of_compatible for binding device node
Modified error messages
Modified sky81452-regulator to return ENODATA when of_node is NULL

v2:
Split the patches for each sub-system
Added 'reg' attribute for I2C address in device tree documents
Added 'compatible' attribute in child drivers
Renamed CONFIG_SKY81452 to CONFIG_MFD_SKY81452
Changed the dependency from I2C=y to I2C, for CONFIG_MFD_SKY81452
Added message for exception or errors.
Added vendor prefix for Skyworks Solutions, Inc.
Add SKY81452 to the Trivial Devices list

Gyungoh Yoo (8):
mfd: Add support for Skyworks SKY81452 driver
backlight: Add support Skyworks SKY81452 backlight driver
devicetree: mfd: Add new SKY81452 mfd binding
devicetree: backlight: Add new SKY81452 backlight binding
devicetree: Add vendor prefix for Skyworks Solutions, Inc.
devicetree: i2c: Add SKY81452 to the Trivial Devices list
regulator: sky81452: Add compatible string for device binding
devicetree: regulator: sky81452: Add compatible string for device
binding

.../devicetree/bindings/i2c/trivial-devices.txt | 1 +
Documentation/devicetree/bindings/mfd/sky81452.txt | 32 ++
.../bindings/regulator/sky81452-regulator.txt | 8 +
.../devicetree/bindings/vendor-prefixes.txt | 1 +
.../video/backlight/sky81452-backlight.txt | 24 ++
drivers/mfd/Kconfig | 12 +
drivers/mfd/Makefile | 1 +
drivers/mfd/sky81452.c | 110 +++++++
drivers/regulator/Kconfig | 2 +-
drivers/regulator/sky81452-regulator.c | 23 +-
drivers/video/backlight/Kconfig | 10 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/sky81452-backlight.c | 344 +++++++++++++++++++++
include/linux/mfd/sky81452.h | 32 ++
include/linux/sky81452-backlight.h | 47 +++
15 files changed, 640 insertions(+), 8 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/sky81452.txt
create mode 100644 Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt
create mode 100644 drivers/mfd/sky81452.c
create mode 100644 drivers/video/backlight/sky81452-backlight.c
create mode 100644 include/linux/mfd/sky81452.h
create mode 100644 include/linux/sky81452-backlight.h

Gyungoh Yoo

2014-10-13 05:47:35 UTC

Signed-off-by: Gyungoh Yoo <***@skyworksinc.com>
---
Changes v4:
Removed MODULE_VERSION()
Modified license to GPLv2

Changes v3:
Fixed the backlight name from 'sky81452-bl' to 'sky81452-backlight'
Assigned mfd_cell.of_compatible for binding device node
Modified error messages

Changes v2:
Renamed CONFIG_SKY81452 to CONFIG_MFD_SKY81452
Changed the dependency from I2C=y to I2C, for CONFIG_MFD_SKY81452
Added message for exception or errors

drivers/mfd/Kconfig | 12 +++++
drivers/mfd/Makefile | 1 +
drivers/mfd/sky81452.c | 110 +++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/sky81452.h | 32 +++++++++++++
4 files changed, 155 insertions(+)
create mode 100644 drivers/mfd/sky81452.c
create mode 100644 include/linux/mfd/sky81452.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index cf66ef1..f62a4a4 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -641,6 +641,18 @@ config MFD_SM501_GPIO
lines on the SM501. The platform data is used to supply the
base number for the first GPIO line to register.

+config MFD_SKY81452
+ tristate "Skyworks Solutions SKY81452"
+ select MFD_CORE
+ select REGMAP_I2C
+ depends on I2C
+ help
+ This is the core driver for the Skyworks SKY81452 backlight and
+ voltage regulator device.
+
+ This driver can also be built as a module. If so, the module
+ will be called sky81452.
+
config MFD_SMSC
bool "SMSC ECE1099 series chips"
depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index d58068a..2962396 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -170,6 +170,7 @@ obj-$(CONFIG_MFD_AS3722) += as3722.o
obj-$(CONFIG_MFD_STW481X) += stw481x.o
obj-$(CONFIG_MFD_IPAQ_MICRO) += ipaq-micro.o
obj-$(CONFIG_MFD_MENF21BMC) += menf21bmc.o
+obj-$(CONFIG_MFD_SKY81452) += sky81452.o

intel-soc-pmic-objs := intel_soc_pmic_core.o intel_soc_pmic_crc.o
obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
diff --git a/drivers/mfd/sky81452.c b/drivers/mfd/sky81452.c
new file mode 100644
index 0000000..bdd3253
--- /dev/null
+++ b/drivers/mfd/sky81452.c
@@ -0,0 +1,110 @@
+/*
+ * sky81452.c SKY81452 MFD driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo <***@skyworksinc.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/sky81452.h>
+
+static const struct regmap_config sky81452_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+};
+
+static int sky81452_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &client->dev;
+ const struct sky81452_platform_data *pdata = dev_get_platdata(dev);
+ struct mfd_cell cells[2];
+ struct regmap *regmap;
+ int ret;
+
+ if (!pdata) {
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+ }
+
+ regmap = devm_regmap_init_i2c(client, &sky81452_config);
+ if (IS_ERR(regmap)) {
+ dev_err(dev, "failed to initialize. err=%ld", PTR_ERR(regmap));
+ return PTR_ERR(regmap);
+ }
+
+ i2c_set_clientdata(client, regmap);
+
+ memset(cells, 0, sizeof(cells));
+ cells[0].name = "sky81452-backlight";
+ cells[0].of_compatible = "skyworks,sky81452-backlight";
+ cells[0].platform_data = pdata->bl_pdata;
+ cells[0].pdata_size = sizeof(*pdata->bl_pdata);
+ cells[1].name = "sky81452-regulator";
+ cells[1].of_compatible = "skyworks,sky81452-regulator";
+ cells[1].platform_data = pdata->regulator_init_data;
+ cells[1].pdata_size = sizeof(*pdata->regulator_init_data);
+
+ ret = mfd_add_devices(dev, -1, cells, ARRAY_SIZE(cells), NULL, 0, NULL);
+ if (ret)
+ dev_err(dev, "failed to add child devices. err=%d", ret);
+
+ return ret;
+}
+
+static int sky81452_remove(struct i2c_client *client)
+{
+ mfd_remove_devices(&client->dev);
+ return 0;
+}
+
+static const struct i2c_device_id sky81452_ids[] = {
+ { "sky81452" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, sky81452_ids);
+
+#ifdef CONFIG_OF
+static const struct of_device_id sky81452_of_match[] = {
+ { .compatible = "skyworks,sky81452", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sky81452_of_match);
+#endif
+
+static struct i2c_driver sky81452_driver = {
+ .driver = {
+ .name = "sky81452",
+ .of_match_table = of_match_ptr(sky81452_of_match),
+ },
+ .probe = sky81452_probe,
+ .remove = sky81452_remove,
+ .id_table = sky81452_ids,
+};
+
+module_i2c_driver(sky81452_driver);
+
+MODULE_DESCRIPTION("Skyworks SKY81452 MFD driver");
+MODULE_AUTHOR("Gyungoh Yoo <***@skyworksinc.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/sky81452.h b/include/linux/mfd/sky81452.h
new file mode 100644
index 0000000..142f762
--- /dev/null
+++ b/include/linux/mfd/sky81452.h
@@ -0,0 +1,32 @@
+/*
+ * sky81452.h SKY81452 MFD driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo <***@skyworksinc.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _SKY81452_H
+#define _SKY81452_H
+
+#include <linux/sky81452-backlight.h>
+#include <linux/regulator/machine.h>
+
+struct sky81452_platform_data {
+ struct sky81452_bl_platform_data *bl_pdata;
+ struct regulator_init_data *regulator_init_data;
+};
+
+#endif

--
1.9.1

Gyungoh Yoo

2014-10-13 05:47:36 UTC

Signed-off-by: Gyungoh Yoo <***@skyworksinc.com>
---
Changes v4:
Reordering header files for readability
Removed calling to backlight_device_unregister()
- because devm_backlight_device_register() was used
Removed MODULE_VERSION()
Modified license to GPLv2

Changes v3:
Modified DBG messages

Changes v2:
Added 'compatible' attribute in the driver
Added message for exception or errors

drivers/video/backlight/Kconfig | 10 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/sky81452-backlight.c | 344 +++++++++++++++++++++++++++
include/linux/sky81452-backlight.h | 47 ++++
4 files changed, 402 insertions(+)
create mode 100644 drivers/video/backlight/sky81452-backlight.c
create mode 100644 include/linux/sky81452-backlight.h

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 8d03924..2586fdd 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -409,6 +409,16 @@ config BACKLIGHT_PANDORA
If you have a Pandora console, say Y to enable the
backlight driver.

+config BACKLIGHT_SKY81452
+ tristate "Backlight driver for SKY81452"
+ depends on BACKLIGHT_CLASS_DEVICE && MFD_SKY81452
+ help
+ If you have a Skyworks SKY81452, say Y to enable the
+ backlight driver.
+
+ To compile this driver as a module, choose M here: the module will
+ be called sky81452-backlight
+
config BACKLIGHT_TPS65217
tristate "TPS65217 Backlight"
depends on BACKLIGHT_CLASS_DEVICE && MFD_TPS65217
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index fcd50b73..d67073f 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_BACKLIGHT_PANDORA) += pandora_bl.o
obj-$(CONFIG_BACKLIGHT_PCF50633) += pcf50633-backlight.o
obj-$(CONFIG_BACKLIGHT_PWM) += pwm_bl.o
obj-$(CONFIG_BACKLIGHT_SAHARA) += kb3886_bl.o
+obj-$(CONFIG_BACKLIGHT_SKY81452) += sky81452-backlight.o
obj-$(CONFIG_BACKLIGHT_TOSA) += tosa_bl.o
obj-$(CONFIG_BACKLIGHT_TPS65217) += tps65217_bl.o
obj-$(CONFIG_BACKLIGHT_WM831X) += wm831x_bl.o
diff --git a/drivers/video/backlight/sky81452-backlight.c b/drivers/video/backlight/sky81452-backlight.c
new file mode 100644
index 0000000..3632e29
--- /dev/null
+++ b/drivers/video/backlight/sky81452-backlight.c
@@ -0,0 +1,344 @@
+/*
+ * sky81452-backlight.c SKY81452 backlight driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo <***@skyworksinc.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/backlight.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/sky81452-backlight.h>
+#include <linux/slab.h>
+
+/* registers */
+#define SKY81452_REG0 0x00
+#define SKY81452_REG1 0x01
+#define SKY81452_REG2 0x02
+#define SKY81452_REG4 0x04
+#define SKY81452_REG5 0x05
+
+/* bit mask */
+#define SKY81452_CS 0xFF
+#define SKY81452_EN 0x3F
+#define SKY81452_IGPW 0x20
+#define SKY81452_PWMMD 0x10
+#define SKY81452_PHASE 0x08
+#define SKY81452_ILIM 0x04
+#define SKY81452_VSHRT 0x03
+#define SKY81452_OCP 0x80
+#define SKY81452_OTMP 0x40
+#define SKY81452_SHRT 0x3F
+#define SKY81452_OPN 0x3F
+
+#define SKY81452_DEFAULT_NAME "lcd-backlight"
+#define SKY81452_MAX_BRIGHTNESS (SKY81452_CS + 1)
+
+#define CTZ(b) __builtin_ctz(b)
+
+static int sky81452_bl_update_status(struct backlight_device *bd)
+{
+ const struct sky81452_bl_platform_data *pdata =
+ dev_get_platdata(bd->dev.parent);
+ const unsigned int brightness = (unsigned int)bd->props.brightness;
+ struct regmap *regmap = bl_get_data(bd);
+ int ret;
+
+ if (brightness > 0) {
+ ret = regmap_write(regmap, SKY81452_REG0, brightness - 1);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ return regmap_update_bits(regmap, SKY81452_REG1, SKY81452_EN,
+ pdata->enable << CTZ(SKY81452_EN));
+ }
+
+ return regmap_update_bits(regmap, SKY81452_REG1, SKY81452_EN, 0);
+}
+
+static int sky81452_bl_get_brightness(struct backlight_device *bd)
+{
+ return bd->props.brightness;
+}
+
+static const struct backlight_ops sky81452_bl_ops = {
+ .update_status = sky81452_bl_update_status,
+ .get_brightness = sky81452_bl_get_brightness,
+};
+
+static ssize_t sky81452_bl_store_enable(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct regmap *regmap = bl_get_data(to_backlight_device(dev));
+ unsigned long value;
+ int ret;
+
+ ret = kstrtoul(buf, 16, &value);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ ret = regmap_update_bits(regmap, SKY81452_REG1, SKY81452_EN,
+ value << CTZ(SKY81452_EN));
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ return count;
+}
+
+static ssize_t sky81452_bl_show_open_short(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct regmap *regmap = bl_get_data(to_backlight_device(dev));
+ unsigned int reg, value = 0;
+ char tmp[3];
+ int i, ret;
+
+ reg = !strcmp(attr->attr.name, "open") ? SKY81452_REG5 : SKY81452_REG4;
+ ret = regmap_read(regmap, reg, &value);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ if (value & SKY81452_SHRT) {
+ *buf = 0;
+ for (i = 0; i < 6; i++) {
+ if (value & 0x01) {
+ sprintf(tmp, "%d ", i + 1);
+ strcat(buf, tmp);
+ }
+ value >>= 1;
+ }
+ strcat(buf, "\n");
+ } else {
+ strcpy(buf, "none\n");
+ }
+
+ return strlen(buf);
+}
+
+static ssize_t sky81452_bl_show_fault(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct regmap *regmap = bl_get_data(to_backlight_device(dev));
+ unsigned int value = 0;
+ int ret;
+
+ ret = regmap_read(regmap, SKY81452_REG4, &value);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
+ *buf = 0;
+
+ if (value & SKY81452_OCP)
+ strcat(buf, "over-current ");
+
+ if (value & SKY81452_OTMP)
+ strcat(buf, "over-temperature");
+
+ strcat(buf, "\n");
+ return strlen(buf);
+}
+
+static DEVICE_ATTR(enable, S_IWGRP | S_IWUSR, NULL, sky81452_bl_store_enable);
+static DEVICE_ATTR(open, S_IRUGO, sky81452_bl_show_open_short, NULL);
+static DEVICE_ATTR(short, S_IRUGO, sky81452_bl_show_open_short, NULL);
+static DEVICE_ATTR(fault, S_IRUGO, sky81452_bl_show_fault, NULL);
+
+static struct attribute *sky81452_bl_attribute[] = {
+ &dev_attr_enable.attr,
+ &dev_attr_open.attr,
+ &dev_attr_short.attr,
+ &dev_attr_fault.attr,
+ NULL
+};
+
+static const struct attribute_group sky81452_bl_attr_group = {
+ .attrs = sky81452_bl_attribute,
+};
+
+#ifdef CONFIG_OF
+static struct sky81452_bl_platform_data *sky81452_bl_parse_dt(
+ struct device *dev)
+{
+ struct device_node *np = of_node_get(dev->of_node);
+ struct sky81452_bl_platform_data *pdata;
+ int ret;
+
+ if (!np) {
+ dev_err(dev, "backlight node not found");
+ return ERR_PTR(-ENODATA);
+ }
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata) {
+ of_node_put(np);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ of_property_read_string(np, "name", &pdata->name);
+ pdata->ignore_pwm = of_property_read_bool(np, "ignore-pwm");
+ pdata->dpwm_mode = of_property_read_bool(np, "dpwm-mode");
+ pdata->phase_shift = of_property_read_bool(np, "phase-shift");
+
+ pdata->gpio_enable = of_get_named_gpio(np, "gpio-enable", 0);
+ if (IS_ERR_VALUE(pdata->gpio_enable))
+ pdata->gpio_enable = -1;
+
+ ret = of_property_read_u32(np, "enable", &pdata->enable);
+ if (IS_ERR_VALUE(ret))
+ pdata->enable = SKY81452_EN >> CTZ(SKY81452_EN);
+
+ ret = of_property_read_u32(np, "short-detection-threshold",
+ &pdata->short_detection_threshold);
+ if (IS_ERR_VALUE(ret))
+ pdata->short_detection_threshold = 7;
+
+ ret = of_property_read_u32(np, "boost-current-limit",
+ &pdata->boost_current_limit);
+ if (IS_ERR_VALUE(ret))
+ pdata->boost_current_limit = 2750;
+
+ of_node_put(np);
+ return pdata;
+}
+#else
+static struct sky81452_bl_platform_data *sky81452_bl_parse_dt(
+ struct device *dev)
+{
+ return ERR_PTR(-EINVAL);
+}
+#endif
+
+static int sky81452_bl_init_device(struct regmap *regmap,
+ struct sky81452_bl_platform_data *pdata)
+{
+ unsigned int value;
+
+ value = pdata->ignore_pwm ? SKY81452_IGPW : 0;
+ value |= pdata->dpwm_mode ? SKY81452_PWMMD : 0;
+ value |= pdata->phase_shift ? 0 : SKY81452_PHASE;
+
+ if (pdata->boost_current_limit == 2300)
+ value |= SKY81452_ILIM;
+ else if (pdata->boost_current_limit != 2720)
+ return -EINVAL;
+
+ if (pdata->short_detection_threshold < 4 ||
+ pdata->short_detection_threshold > 7)
+ return -EINVAL;
+ value |= (7 - pdata->short_detection_threshold) << CTZ(SKY81452_VSHRT);
+
+ return regmap_write(regmap, SKY81452_REG2, value);
+}
+
+static int sky81452_bl_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct regmap *regmap = dev_get_drvdata(dev->parent);
+ struct sky81452_bl_platform_data *pdata = dev_get_platdata(dev);
+ struct backlight_device *bd;
+ struct backlight_properties props;
+ const char *name;
+ int ret;
+
+ if (!pdata) {
+ pdata = sky81452_bl_parse_dt(dev);
+ if (IS_ERR(pdata))
+ return PTR_ERR(pdata);
+ }
+
+ if (pdata->gpio_enable >= 0) {
+ ret = devm_gpio_request_one(dev, pdata->gpio_enable,
+ GPIOF_OUT_INIT_HIGH, "sky81452-en");
+ if (IS_ERR_VALUE(ret)) {
+ dev_err(dev, "failed to request GPIO. err=%d", ret);
+ return ret;
+ }
+ }
+
+ ret = sky81452_bl_init_device(regmap, pdata);
+ if (IS_ERR_VALUE(ret)) {
+ dev_err(dev, "failed to initialize. err=%d", ret);
+ return ret;
+ }
+
+ memset(&props, 0, sizeof(props));
+ props.max_brightness = SKY81452_MAX_BRIGHTNESS,
+ name = pdata->name ? pdata->name : SKY81452_DEFAULT_NAME;
+ bd = devm_backlight_device_register(dev, name, dev, regmap,
+ &sky81452_bl_ops, &props);
+ if (IS_ERR(bd)) {
+ dev_err(dev, "failed to register. err=%ld", PTR_ERR(bd));
+ return PTR_ERR(bd);
+ }
+
+ platform_set_drvdata(pdev, bd);
+
+ ret = sysfs_create_group(&bd->dev.kobj, &sky81452_bl_attr_group);
+ if (IS_ERR_VALUE(ret)) {
+ dev_err(dev, "failed to create attribute. err=%d", ret);
+ return ret;
+ }
+
+ return ret;
+}
+
+static int sky81452_bl_remove(struct platform_device *pdev)
+{
+ const struct sky81452_bl_platform_data *pdata =
+ dev_get_platdata(&pdev->dev);
+ struct backlight_device *bd = platform_get_drvdata(pdev);
+
+ sysfs_remove_group(&bd->dev.kobj, &sky81452_bl_attr_group);
+
+ bd->props.power = FB_BLANK_UNBLANK;
+ bd->props.brightness = 0;
+ backlight_update_status(bd);
+
+ if (pdata->gpio_enable >= 0)
+ gpio_set_value_cansleep(pdata->gpio_enable, 0);
+
+ return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id sky81452_bl_of_match[] = {
+ { .compatible = "skyworks,sky81452-backlight", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sky81452_bl_of_match);
+#endif
+
+static struct platform_driver sky81452_bl_driver = {
+ .driver = {
+ .name = "sky81452-backlight",
+ .of_match_table = of_match_ptr(sky81452_bl_of_match),
+ },
+ .probe = sky81452_bl_probe,
+ .remove = sky81452_bl_remove,
+};
+
+module_platform_driver(sky81452_bl_driver);
+
+MODULE_DESCRIPTION("Skyworks SKY81452 backlight driver");
+MODULE_AUTHOR("Gyungoh Yoo <***@skyworksinc.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/sky81452-backlight.h b/include/linux/sky81452-backlight.h
new file mode 100644
index 0000000..b3635f9
--- /dev/null
+++ b/include/linux/sky81452-backlight.h
@@ -0,0 +1,47 @@
+/*
+ * sky81452.h SKY81452 backlight driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo <***@skyworksinc.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _SKY81452_BACKLIGHT_H
+#define _SKY81452_BACKLIGHT_H
+
+/**
+ * struct sky81452_platform_data
+ * @name: backlight driver name.
+ If it is not defined, default name is lcd-backlight.
+ * @gpio_enable:GPIO number which control EN pin
+ * @enable: Enable mask for current sink channel 1, 2, 3, 4, 5 and 6.
+ * @ignore_pwm: true if DPWMI should be ignored.
+ * @dpwm_mode: true is DPWM dimming mode, otherwise Analog dimming mode.
+ * @phase_shift:true is phase shift mode.
+ * @short_detecion_threshold: It should be one of 4, 5, 6 and 7V.
+ * @boost_current_limit: It should be one of 2300, 2750mA.
+ */
+struct sky81452_bl_platform_data {
+ const char *name;
+ int gpio_enable;
+ unsigned int enable;
+ bool ignore_pwm;
+ bool dpwm_mode;
+ bool phase_shift;
+ unsigned int short_detection_threshold;
+ unsigned int boost_current_limit;
+};
+
+#endif

--
1.9.1

Gyungoh Yoo

2014-10-13 05:47:37 UTC

Signed-off-by: Gyungoh Yoo <***@skyworksinc.com>
---
Changes v4:
Nothing

Changes v3:
Nothing

Changes v2:
Added reg attribute for I2C slave address

Documentation/devicetree/bindings/mfd/sky81452.txt | 32 ++++++++++++++++++++++
1 file changed, 32 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/sky81452.txt

diff --git a/Documentation/devicetree/bindings/mfd/sky81452.txt b/Documentation/devicetree/bindings/mfd/sky81452.txt
new file mode 100644
index 0000000..d61904a
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/sky81452.txt
@@ -0,0 +1,32 @@
+SKY81452 bindings
+
+Required properties:
+- compatible : Must be "skyworks,sky81452"
+- reg : I2C slave address
+
+Required child nodes:
+- backlight : container node for backlight following the binding
+ in video/backlight/sky81452-backlight.txt
+- regulator : container node for regulators following the binding
+ in regulator/sky81452-regulator.txt
+
+Example:
+
+ ***@2c {
+ compatible = "skyworks,sky81452";
+ reg = <0x2c>;
+
+ backlight {
+ compatible = "skyworks,sky81452-backlight";
+ name = "pwm-backlight";
+ enable = <0x3F>;
+ ignore-pwm;
+ };
+
+ regulator {
+ compatible = "skyworks,sky81452-regulator";
+ regulator-name = "touch_en";
+ regulator-min-microvolt = <4500000>;
+ regulator-max-microvolt = <8000000>;
+ };
+ };

--
1.9.1

Gyungoh Yoo

2014-10-13 05:47:38 UTC

Signed-off-by: Gyungoh Yoo <***@skyworksinc.com>
---
Changes v4:
Nothing

Changes v3:
Nothing

Changes v2:
Added reg attribute for I2C slave address

.../video/backlight/sky81452-backlight.txt | 24 ++++++++++++++++++++++
1 file changed, 24 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt

diff --git a/Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt b/Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt
new file mode 100644
index 0000000..2c2d947
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/backlight/sky81452-backlight.txt
@@ -0,0 +1,24 @@
+SKY81452-backlight bindings
+
+Required properties:
+- compatible : Must be "skyworks,sky81452-backlight"
+
+Optional properties:
+- name : Name of backlight device. Default is 'lcd-backlight'.
+- gpio-enable : GPIO to use to EN pin.
+- enable : Enable mask for current sink channel 1 to 6.
+- ignore-pwm : Ignore both PWM input
+- dpwm-mode : Enable DPWM dimming mode, otherwise Analog dimming mode
+- phase-shift : Enable phase shift mode
+- ovp-level : Over-voltage protection level. Should be between 14 or 28V.
+- short-detection-threshold : It should be one of 4, 5, 6 and 7V.
+- boost-current-limit : It should be one of 800, 1100 and 1500mA.
+
+Example:
+
+ backlight {
+ compatible = "skyworks,sky81452-backlight";
+ name = "pwm-backlight";
+ enable = <0x3F>;
+ ignore-pwm;
+ };

--
1.9.1

Gyungoh Yoo

2014-10-13 05:47:39 UTC

Signed-off-by: Gyungoh Yoo <***@skyworksinc.com>
---
Changes v4:
Nothing

Changes v3:
Nothing

Changes v2:
Added vendor prefix for Skyworks Solutions, Inc.

Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index f67e3f8..534a583 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -136,6 +136,7 @@ sii Seiko Instruments, Inc.
silergy Silergy Corp.
sirf SiRF Technology, Inc.
sitronix Sitronix Technology Corporation
+skyworks Skyworks Solutions, Inc.
smsc Standard Microsystems Corporation
snps Synopsys, Inc.
solidrun SolidRun

--
1.9.1

Gyungoh Yoo

2014-10-13 05:47:40 UTC

Signed-off-by: Gyungoh Yoo <***@skyworksinc.com>
---
Changes v4:
Nothing

Changes v3:
Nothing

Changes v2:
Add SKY81452 to the Trivial Devices list

Documentation/devicetree/bindings/i2c/trivial-devices.txt | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index 5af3d9d..2ed3baa 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -78,6 +78,7 @@ ramtron,24c64 i2c serial eeprom (24cxx)
ricoh,rs5c372a I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
samsung,24ad0xd1 S524AD0XF1 (128K/256K-bit Serial EEPROM for Low Power)
sii,s35390a 2-wire CMOS real-time clock
+skyworks,sky81452 Skyworks SKY81452: Six-Channel White LED Driver with Touch Panel Bias Supply
st-micro,24c256 i2c serial eeprom (24cxx)
stm,m41t00 Serial Access TIMEKEEPER
stm,m41t62 Serial real-time clock (RTC) with alarm

--
1.9.1

Gyungoh Yoo

2014-10-13 05:47:41 UTC

Signed-off-by: Gyungoh Yoo <***@skyworksinc.com>
---
Changes v4:
Removed MODULE_VERSION()
Modified license to GPLv2

Changes v3:
Modified to return ENODATA when of_node is NULL
Modified the messages in error cases

Changes v2:
Added 'compatible' attribute in the driver
Added message for exception or errors.

drivers/regulator/Kconfig | 2 +-
drivers/regulator/sky81452-regulator.c | 23 ++++++++++++++++-------
2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 55d7b7b..fe84bd4 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -547,7 +547,7 @@ config REGULATOR_S5M8767

config REGULATOR_SKY81452
tristate "Skyworks Solutions SKY81452 voltage regulator"
- depends on SKY81452
+ depends on MFD_SKY81452
help
This driver supports Skyworks SKY81452 voltage output regulator
via I2C bus. SKY81452 has one voltage linear regulator can be
diff --git a/drivers/regulator/sky81452-regulator.c b/drivers/regulator/sky81452-regulator.c
index 97aff0c..dfdefdb 100644
--- a/drivers/regulator/sky81452-regulator.c
+++ b/drivers/regulator/sky81452-regulator.c
@@ -68,12 +68,11 @@ static const struct regulator_desc sky81452_reg = {
static struct regulator_init_data *sky81452_reg_parse_dt(struct device *dev)
{
struct regulator_init_data *init_data;
- struct device_node *np;
+ struct device_node *np = of_node_get(dev->of_node);

- np = of_get_child_by_name(dev->parent->of_node, "regulator");
- if (unlikely(!np)) {
+ if (!np) {
dev_err(dev, "regulator node not found");
- return NULL;
+ return ERR_PTR(-ENODATA);
}

init_data = of_get_regulator_init_data(dev, np);
@@ -107,17 +106,28 @@ static int sky81452_reg_probe(struct platform_device *pdev)
config.regmap = dev_get_drvdata(dev->parent);

rdev = devm_regulator_register(dev, &sky81452_reg, &config);
- if (IS_ERR(rdev))
+ if (IS_ERR(rdev)) {
+ dev_err(dev, "failed to register. err=%ld", PTR_ERR(rdev));
return PTR_ERR(rdev);
+ }

platform_set_drvdata(pdev, rdev);

return 0;
}

+#ifdef CONFIG_OF
+static const struct of_device_id sky81452_reg_of_match[] = {
+ { .compatible = "skyworks,sky81452-regulator", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sky81452_reg_of_match);
+#endif
+
static struct platform_driver sky81452_reg_driver = {
.driver = {
.name = "sky81452-regulator",
+ .of_match_table = of_match_ptr(sky81452_reg_of_match),
},
.probe = sky81452_reg_probe,
};
@@ -126,5 +136,4 @@ module_platform_driver(sky81452_reg_driver);

MODULE_DESCRIPTION("Skyworks SKY81452 Regulator driver");
MODULE_AUTHOR("Gyungoh Yoo <***@skyworksinc.com>");
-MODULE_LICENSE("GPL");
-MODULE_VERSION("1.0");
+MODULE_LICENSE("GPL v2");

--
1.9.1

Mark Brown

2014-10-13 10:35:12 UTC

If you're doing anything here you should convert the code to use the
simplified interface for parsing regulator data based on putting the
subnode name and regualtor name in the regulator descriptor.

Gyungoh Yoo

2014-10-14 01:11:12 UTC

If you're doing anything here you should convert the code to use the
simplified interface for parsing regulator data based on putting the
subnode name and regualtor name in the regulator descriptor.

Thank you for the review.

The driver has of_device_id.

static const struct of_device_id sky81452_reg_of_match[] = {
{ .compatible = "skyworks,sky81452-regulator", },
{ }
};

And the MFD adds the driver with of_compatible="skyworks,sky81452-regulator".
MFD core maps the device node and the regulator driver does
not need to parse the subnode name.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Mark Brown

2014-10-14 07:56:06 UTC

Post by Mark Brown
If you're doing anything here you should convert the code to use the
simplified interface for parsing regulator data based on putting the
subnode name and regualtor name in the regulator descriptor.

No, that's for the device (and as I say I don't think this is a good
idea). I'm talking about the parsing of the constraints from the DT.

Gyungoh Yoo

2014-10-17 09:19:50 UTC

No, that's for the device (and as I say I don't think this is a good
idea). I'm talking about the parsing of the constraints from the DT.

Wow. I understand.
I missed .supply_name. I think I need something like
below, before devm_regulator_register().

config->supply_name = init_data->constraints.name;

I will resubmit it.
Thank you so much.

Mark Brown

2014-10-17 10:17:34 UTC

Post by Gyungoh Yoo
Wow. I understand.
I missed .supply_name. I think I need something like
below, before devm_regulator_register().
config->supply_name = init_data->constraints.name;
I will resubmit it.

No, not supply_name (though you really should be initializing that
anyway) and you definitely don't want to set supply_name to something
from constraints. The supply_name should be the name of the input pin
that supplies power to the regulator. I was referring to of_match and
regulators_node.

Gyungoh Yoo

2014-10-17 10:43:09 UTC

Wow. I had misunderstanding.

Are you talking about simplification using of_regulator_match()?
This driver has only one regulator.
Is the API also useful for this driver?
Like reg-fixed-voltage, how about using of_get_fixed_voltage_config()?

Thanks.

Mark Brown

2014-10-17 14:26:05 UTC

Post by Gyungoh Yoo
Are you talking about simplification using of_regulator_match()?
This driver has only one regulator.
Is the API also useful for this driver?

The thing I'm seeing is that the binding for your device with the
subnode looks very much like the device trees of devices with multiple
regulators. The fact that you only have one regulator is a bit
difference but not that much. It seems like drivers should fit into one
of two patterns: either the regulator is described in the root node for
the device for single purpose devices or there should be a collection of
regulators like is supported with this helper API. Having a collection
with only one node doesn't seem to be a problem in any way.

Post by Gyungoh Yoo
Like reg-fixed-voltage, how about using of_get_fixed_voltage_config()?

The driver doesn't seem to need any property parsing of its own so it
shoudn't need anything beyond basic calls into the core.

Gyungoh Yoo

2014-10-20 06:10:24 UTC

Post by Gyungoh Yoo
Are you talking about simplification using of_regulator_match()?
This driver has only one regulator.
Is the API also useful for this driver?

Post by Gyungoh Yoo
Like reg-fixed-voltage, how about using of_get_fixed_voltage_config()?

The driver doesn't seem to need any property parsing of its own so it
shoudn't need anything beyond basic calls into the core.

Thank you for your kind comments.
My understanding is getting better.

For my clear understanding:
I think the original designed which I wanted to design is similar
with arizona-ldo1.c
It seems that this is 1st pattern your explained above.
Can I ask what is different between arizona-ldo1.c and
this sky81452-regulator.c?
I think both are designed under root node.

Thank you.

Gyungoh Yoo

2014-10-13 05:47:42 UTC