Re: [PATCH 1/3] Adding Skyworks SKY81452 MFD driver
On Wed, Aug 27, 2014 at 09:39:21AM +0100, Lee Jones wrote:
> On Wed, 27 Aug 2014, Gyungoh Yoo wrote:
> > On Tue, Aug 26, 2014 at 09:22:58AM +0100, Lee Jones wrote:
> > > On Mon, 25 Aug 2014, Gyungoh Yoo wrote:
> > > > On Thu, Aug 21, 2014 at 10:45:02AM +0100, Lee Jones wrote:
> > > > > 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.
> > > > >
> > > > >
> > > > >
> > > > > > Signed-off-by: Gyungoh Yoo
> > > > > > ---
> > >
> > > [...]
> > >
> > > > > > +static int sky81452_register_devices(struct device *dev,
> > > > > > + const struct sky81452_platform_data *pdata)
> > > > > > +{
> > > > > > + struct mfd_cell cells[] = {
> > > > > > + {
> > > > > > + .name = "sky81452-bl",
> > > > > > + .platform_data = pdata->bl_pdata,
> > > > > > + .pdata_size = sizeof(*pdata->bl_pdata),
> > > > >
> > > > > 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.
> > > >
> > > > 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?
> >
> > The backlight driver which is one of the child driver is obtain its DT node
> > like this
> >
> > np = 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.
I see. Thank you.
>
> > > [...]
> > >
> > > > > > + 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.
> > > >
> > > > 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?
> >
> > 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 = 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 = dev_get_platdata(dev->parent);
> const struct sky81452_bl_platform_data = pdata = ppdata->bl_pdata;
I think it could be a good way for this. Thank you.
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] Adding Skyworks SKY81452 MFD driver
On Wed, 27 Aug 2014, Gyungoh Yoo wrote:
> On Tue, Aug 26, 2014 at 09:22:58AM +0100, Lee Jones wrote:
> > On Mon, 25 Aug 2014, Gyungoh Yoo wrote:
> > > On Thu, Aug 21, 2014 at 10:45:02AM +0100, Lee Jones wrote:
> > > > 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.
> > > >
> > > >
> > > >
> > > > > Signed-off-by: Gyungoh Yoo
> > > > > ---
> >
> > [...]
> >
> > > > > +static int sky81452_register_devices(struct device *dev,
> > > > > + const struct sky81452_platform_data *pdata)
> > > > > +{
> > > > > + struct mfd_cell cells[] = {
> > > > > + {
> > > > > + .name = "sky81452-bl",
> > > > > + .platform_data = pdata->bl_pdata,
> > > > > + .pdata_size = sizeof(*pdata->bl_pdata),
> > > >
> > > > 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.
> > >
> > > 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?
>
> The backlight driver which is one of the child driver is obtain its DT node
> like this
>
> np = 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.
> > [...]
> >
> > > > > + 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.
> > >
> > > 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?
>
> 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 = 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 = dev_get_platdata(dev->parent);
const struct sky81452_bl_platform_data = pdata = ppdata->bl_pdata;
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] Adding Skyworks SKY81452 MFD driver
On Wed, 27 Aug 2014, Gyungoh Yoo wrote:
On Tue, Aug 26, 2014 at 09:22:58AM +0100, Lee Jones wrote:
On Mon, 25 Aug 2014, Gyungoh Yoo wrote:
On Thu, Aug 21, 2014 at 10:45:02AM +0100, Lee Jones wrote:
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
Signed-off-by: Gyungoh Yoo jack@skyworksinc.com
---
[...]
+static int sky81452_register_devices(struct device *dev,
+ const struct sky81452_platform_data *pdata)
+{
+ struct mfd_cell cells[] = {
+ {
+ .name = sky81452-bl,
+ .platform_data = pdata-bl_pdata,
+ .pdata_size = sizeof(*pdata-bl_pdata),
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.
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?
The backlight driver which is one of the child driver is obtain its DT node
like this
np = 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.
[...]
+ 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.
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?
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 = 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 = dev_get_platdata(dev-parent);
const struct sky81452_bl_platform_data = pdata = ppdata-bl_pdata;
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] Adding Skyworks SKY81452 MFD driver
On Wed, Aug 27, 2014 at 09:39:21AM +0100, Lee Jones wrote:
On Wed, 27 Aug 2014, Gyungoh Yoo wrote:
On Tue, Aug 26, 2014 at 09:22:58AM +0100, Lee Jones wrote:
On Mon, 25 Aug 2014, Gyungoh Yoo wrote:
On Thu, Aug 21, 2014 at 10:45:02AM +0100, Lee Jones wrote:
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
Signed-off-by: Gyungoh Yoo jack@skyworksinc.com
---
[...]
+static int sky81452_register_devices(struct device *dev,
+ const struct sky81452_platform_data *pdata)
+{
+ struct mfd_cell cells[] = {
+ {
+ .name = sky81452-bl,
+ .platform_data = pdata-bl_pdata,
+ .pdata_size = sizeof(*pdata-bl_pdata),
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.
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?
The backlight driver which is one of the child driver is obtain its DT node
like this
np = 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.
I see. Thank you.
[...]
+ 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.
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?
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 = 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 = dev_get_platdata(dev-parent);
const struct sky81452_bl_platform_data = pdata = ppdata-bl_pdata;
I think it could be a good way for this. Thank you.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] Adding Skyworks SKY81452 MFD driver
On Tue, Aug 26, 2014 at 09:22:58AM +0100, Lee Jones wrote:
> On Mon, 25 Aug 2014, Gyungoh Yoo wrote:
> > On Thu, Aug 21, 2014 at 10:45:02AM +0100, Lee Jones wrote:
> > > 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.
> > >
> > >
> > >
> > > > Signed-off-by: Gyungoh Yoo
> > > > ---
>
> [...]
>
> > > > +static int sky81452_register_devices(struct device *dev,
> > > > + const struct sky81452_platform_data *pdata)
> > > > +{
> > > > + struct mfd_cell cells[] = {
> > > > + {
> > > > + .name = "sky81452-bl",
> > > > + .platform_data = pdata->bl_pdata,
> > > > + .pdata_size = sizeof(*pdata->bl_pdata),
> > >
> > > 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.
> >
> > 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?
The backlight driver which is one of the child driver is obtain its DT node
like this
np = of_get_child_by_name(dev->parent->of_node, "backlight");
>
> [...]
>
> > > > + 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.
> >
> > 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?
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 = dev_get_drvdata(dev->parent);
>
> [...]
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] Adding Skyworks SKY81452 MFD driver
On Mon, 25 Aug 2014, Gyungoh Yoo wrote:
> On Thu, Aug 21, 2014 at 10:45:02AM +0100, Lee Jones wrote:
> > 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.
> >
> >
> >
> > > Signed-off-by: Gyungoh Yoo
> > > ---
[...]
> > > +static int sky81452_register_devices(struct device *dev,
> > > + const struct sky81452_platform_data *pdata)
> > > +{
> > > + struct mfd_cell cells[] = {
> > > + {
> > > + .name = "sky81452-bl",
> > > + .platform_data = pdata->bl_pdata,
> > > + .pdata_size = sizeof(*pdata->bl_pdata),
> >
> > 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.
>
> 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);
> >
> > 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?
Then how to you obtain the shared register map you created?
[...]
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] Adding Skyworks SKY81452 MFD driver
On Mon, 25 Aug 2014, Gyungoh Yoo wrote:
On Thu, Aug 21, 2014 at 10:45:02AM +0100, Lee Jones wrote:
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
Signed-off-by: Gyungoh Yoo jack@skyworksinc.com
---
[...]
+static int sky81452_register_devices(struct device *dev,
+ const struct sky81452_platform_data *pdata)
+{
+ struct mfd_cell cells[] = {
+ {
+ .name = sky81452-bl,
+ .platform_data = pdata-bl_pdata,
+ .pdata_size = sizeof(*pdata-bl_pdata),
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.
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);
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?
Then how to you obtain the shared register map you created?
[...]
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] Adding Skyworks SKY81452 MFD driver
On Tue, Aug 26, 2014 at 09:22:58AM +0100, Lee Jones wrote:
On Mon, 25 Aug 2014, Gyungoh Yoo wrote:
On Thu, Aug 21, 2014 at 10:45:02AM +0100, Lee Jones wrote:
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
Signed-off-by: Gyungoh Yoo jack@skyworksinc.com
---
[...]
+static int sky81452_register_devices(struct device *dev,
+ const struct sky81452_platform_data *pdata)
+{
+ struct mfd_cell cells[] = {
+ {
+ .name = sky81452-bl,
+ .platform_data = pdata-bl_pdata,
+ .pdata_size = sizeof(*pdata-bl_pdata),
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.
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?
The backlight driver which is one of the child driver is obtain its DT node
like this
np = of_get_child_by_name(dev-parent-of_node, backlight);
[...]
+ 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.
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?
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 = dev_get_drvdata(dev-parent);
[...]
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] Adding Skyworks SKY81452 MFD driver
Thank you for reviewing the patches.
On Thu, Aug 21, 2014 at 10:45:02AM +0100, Lee Jones wrote:
> 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.
>
>
>
> > Signed-off-by: Gyungoh Yoo
> > ---
> > 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.
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
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/sky81452.txt
> > b/Documentation/devicetree/bindings/mfd/sky81452.txt
> > new file mode 100644
> > index 000..5fb0b4f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/sky81452.txt
> > @@ -0,0 +1,24 @@
> > +SKY81452 bindings
> > +
> > +Required properties:
> > +- compatible : Must be "skyworks,sky81452"
> > +
> > +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:
> > +
> > + sky81452@2C {
>
> Lower case.
>
> > + compatible = "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.
I will update it.
>
> > 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
> > siiSeiko 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.
> >
> > +config SKY81452
>
> MFD_SKY81452
>
> > + tristate "Skyworks Solutions SKY81452"
> > + select MFD_CORE
> > + select REGMAP_I2C
> > + depends on I2C=y
>
> Why do you need I2C to be built-in, rather than as a module?
It will depend only 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..191c656 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_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 000..566912f
> > --- /dev/null
> > +++ b/drivers/mfd/sky81452.c
> > @@ -0,0 +1,113 @@
> > +/*
> > + * sky81452.c SKY81452 MFD driver
> > + *
> > + * Copyright 2014 Skyworks Solutions Inc.
> > + * Author : Gyungoh Yoo
> > + *
> > + * 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
Re: [PATCH 1/3] Adding Skyworks SKY81452 MFD driver
Thank you for reviewing the patches.
On Thu, Aug 21, 2014 at 10:45:02AM +0100, Lee Jones wrote:
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
Signed-off-by: Gyungoh Yoo jack@skyworksinc.com
---
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.
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
diff --git a/Documentation/devicetree/bindings/mfd/sky81452.txt
b/Documentation/devicetree/bindings/mfd/sky81452.txt
new file mode 100644
index 000..5fb0b4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/sky81452.txt
@@ -0,0 +1,24 @@
+SKY81452 bindings
+
+Required properties:
+- compatible : Must be skyworks,sky81452
+
+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:
+
+ sky81452@2C {
Lower case.
+ compatible = 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.
I will update it.
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
siiSeiko 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.
+config SKY81452
MFD_SKY81452
+ tristate Skyworks Solutions SKY81452
+ select MFD_CORE
+ select REGMAP_I2C
+ depends on I2C=y
Why do you need I2C to be built-in, rather than as a module?
It will depend only 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..191c656 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_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 000..566912f
--- /dev/null
+++ b/drivers/mfd/sky81452.c
@@ -0,0 +1,113 @@
+/*
+ * sky81452.c SKY81452 MFD driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo jack@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
Re: [PATCH 1/3] Adding Skyworks SKY81452 MFD driver
On Thu, 21 Aug 2014, Thierry Reding wrote:
> 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.
Yes, I suppose it would. That's sad.
> > > +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 ");
> > > +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.
Fair enough. Withdrawn.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] Adding Skyworks SKY81452 MFD driver
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_AUTHOR("Gyungoh Yoo ");
> > +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
pgpTrOz3o2vtO.pgp
Description: PGP signature
Re: [PATCH 1/3] Adding Skyworks SKY81452 MFD driver
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.
> Signed-off-by: Gyungoh Yoo
> ---
> 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.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 000..5fb0b4f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/sky81452.txt
> @@ -0,0 +1,24 @@
> +SKY81452 bindings
> +
> +Required properties:
> +- compatible : Must be "skyworks,sky81452"
> +
> +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:
> +
> + sky81452@2C {
Lower case.
> + compatible = "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 @@ silabsSilicon 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.
>
> +config SKY81452
MFD_SKY81452
> + tristate "Skyworks Solutions SKY81452"
> + select MFD_CORE
> + select REGMAP_I2C
> + depends on I2C=y
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=y
> 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) += as3711.o
> obj-$(CONFIG_MFD_AS3722) += as3722.o
> obj-$(CONFIG_MFD_STW481X)+= stw481x.o
> obj-$(CONFIG_MFD_IPAQ_MICRO) += ipaq-micro.o
> +obj-$(CONFIG_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 000..566912f
> --- /dev/null
> +++ b/drivers/mfd/sky81452.c
> @@ -0,0 +1,113 @@
> +/*
> + * sky81452.cSKY81452 MFD driver
> + *
> + * Copyright 2014 Skyworks Solutions Inc.
> + * Author : Gyungoh Yoo
> + *
> + * 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
Re: [PATCH 1/3] Adding Skyworks SKY81452 MFD driver
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
Signed-off-by: Gyungoh Yoo jack@skyworksinc.com
---
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.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 000..5fb0b4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/sky81452.txt
@@ -0,0 +1,24 @@
+SKY81452 bindings
+
+Required properties:
+- compatible : Must be skyworks,sky81452
+
+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:
+
+ sky81452@2C {
Lower case.
+ compatible = 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 @@ silabsSilicon 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.
+config SKY81452
MFD_SKY81452
+ tristate Skyworks Solutions SKY81452
+ select MFD_CORE
+ select REGMAP_I2C
+ depends on I2C=y
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=y
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) += as3711.o
obj-$(CONFIG_MFD_AS3722) += as3722.o
obj-$(CONFIG_MFD_STW481X)+= stw481x.o
obj-$(CONFIG_MFD_IPAQ_MICRO) += ipaq-micro.o
+obj-$(CONFIG_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 000..566912f
--- /dev/null
+++ b/drivers/mfd/sky81452.c
@@ -0,0 +1,113 @@
+/*
+ * sky81452.cSKY81452 MFD driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo jack@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
Re: [PATCH 1/3] Adding Skyworks SKY81452 MFD driver
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_AUTHOR(Gyungoh Yoo jack@skyworksinc.com);
+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
pgpTrOz3o2vtO.pgp
Description: PGP signature
Re: [PATCH 1/3] Adding Skyworks SKY81452 MFD driver
On Thu, 21 Aug 2014, Thierry Reding wrote:
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.
Yes, I suppose it would. That's sad.
+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@skyworksinc.com);
+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.
Fair enough. Withdrawn.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] Adding Skyworks SKY81452 MFD driver
On Tue, 12 Aug 2014, GyungOh Yoo wrote:
> 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.
> On Fri, Aug 8, 2014 at 6:08 PM, Gyungoh Yoo wrote:
>
> > Signed-off-by: Gyungoh Yoo
> > ---
> > Documentation/devicetree/bindings/mfd/sky81452.txt | 24 +
> > .../devicetree/bindings/vendor-prefixes.txt| 1 +
> > 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
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/sky81452.txt
> > b/Documentation/devicetree/bindings/mfd/sky81452.txt
> > new file mode 100644
> > index 000..5fb0b4f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/sky81452.txt
> > @@ -0,0 +1,24 @@
> > +SKY81452 bindings
> > +
> > +Required properties:
> > +- compatible : Must be "skyworks,sky81452"
> > +
> > +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:
> > +
> > + sky81452@2C {
> > + compatible = "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
> > siiSeiko 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.
> >
> > +config SKY81452
> > + tristate "Skyworks Solutions SKY81452"
> > + select MFD_CORE
> > + select REGMAP_I2C
> > + depends on I2C=y
> > + 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..191c656 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_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 000..566912f
> > --- /dev/null
> > +++ b/drivers/mfd/sky81452.c
> > @@ -0,0 +1,113 @@
> > +/*
> > + * sky81452.c SKY81452 MFD driver
> > + *
> > + * Copyright 2014 Skyworks Solutions Inc.
> > + * Author : Gyungoh Yoo
> > + *
> > + * 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
Re: [PATCH 1/3] Adding Skyworks SKY81452 MFD driver
On Tue, 12 Aug 2014, GyungOh Yoo wrote:
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.
On Fri, Aug 8, 2014 at 6:08 PM, Gyungoh Yoo gyun...@gmail.com wrote:
Signed-off-by: Gyungoh Yoo jack@skyworksinc.com
---
Documentation/devicetree/bindings/mfd/sky81452.txt | 24 +
.../devicetree/bindings/vendor-prefixes.txt| 1 +
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
diff --git a/Documentation/devicetree/bindings/mfd/sky81452.txt
b/Documentation/devicetree/bindings/mfd/sky81452.txt
new file mode 100644
index 000..5fb0b4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/sky81452.txt
@@ -0,0 +1,24 @@
+SKY81452 bindings
+
+Required properties:
+- compatible : Must be skyworks,sky81452
+
+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:
+
+ sky81452@2C {
+ compatible = 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
siiSeiko 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.
+config SKY81452
+ tristate Skyworks Solutions SKY81452
+ select MFD_CORE
+ select REGMAP_I2C
+ depends on I2C=y
+ 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..191c656 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_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 000..566912f
--- /dev/null
+++ b/drivers/mfd/sky81452.c
@@ -0,0 +1,113 @@
+/*
+ * sky81452.c SKY81452 MFD driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo jack@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
Re: [PATCH 1/3] Adding Skyworks SKY81452 MFD driver
Hello,
Can somebody please review if the code has anything to improve?
Thanks.
On Fri, Aug 08, 2014 at 06:08:19PM +0900, Gyungoh Yoo wrote:
> Signed-off-by: Gyungoh Yoo
> ---
> Documentation/devicetree/bindings/mfd/sky81452.txt | 24 +
> .../devicetree/bindings/vendor-prefixes.txt| 1 +
> 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
>
> diff --git a/Documentation/devicetree/bindings/mfd/sky81452.txt
> b/Documentation/devicetree/bindings/mfd/sky81452.txt
> new file mode 100644
> index 000..5fb0b4f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/sky81452.txt
> @@ -0,0 +1,24 @@
> +SKY81452 bindings
> +
> +Required properties:
> +- compatible : Must be "skyworks,sky81452"
> +
> +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:
> +
> + sky81452@2C {
> + compatible = "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 @@ silabsSilicon 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.
>
> +config SKY81452
> + tristate "Skyworks Solutions SKY81452"
> + select MFD_CORE
> + select REGMAP_I2C
> + depends on I2C=y
> + 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..191c656 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_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 000..566912f
> --- /dev/null
> +++ b/drivers/mfd/sky81452.c
> @@ -0,0 +1,113 @@
> +/*
> + * sky81452.cSKY81452 MFD driver
> + *
> + * Copyright 2014 Skyworks Solutions Inc.
> + * Author : Gyungoh Yoo
> + *
> + * 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
Re: [PATCH 1/3] Adding Skyworks SKY81452 MFD driver
Hello,
Can somebody please review if the code has anything to improve?
Thanks.
On Fri, Aug 08, 2014 at 06:08:19PM +0900, Gyungoh Yoo wrote:
Signed-off-by: Gyungoh Yoo jack@skyworksinc.com
---
Documentation/devicetree/bindings/mfd/sky81452.txt | 24 +
.../devicetree/bindings/vendor-prefixes.txt| 1 +
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
diff --git a/Documentation/devicetree/bindings/mfd/sky81452.txt
b/Documentation/devicetree/bindings/mfd/sky81452.txt
new file mode 100644
index 000..5fb0b4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/sky81452.txt
@@ -0,0 +1,24 @@
+SKY81452 bindings
+
+Required properties:
+- compatible : Must be skyworks,sky81452
+
+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:
+
+ sky81452@2C {
+ compatible = 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 @@ silabsSilicon 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.
+config SKY81452
+ tristate Skyworks Solutions SKY81452
+ select MFD_CORE
+ select REGMAP_I2C
+ depends on I2C=y
+ 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..191c656 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_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 000..566912f
--- /dev/null
+++ b/drivers/mfd/sky81452.c
@@ -0,0 +1,113 @@
+/*
+ * sky81452.cSKY81452 MFD driver
+ *
+ * Copyright 2014 Skyworks Solutions Inc.
+ * Author : Gyungoh Yoo jack@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_register_devices(struct device *dev,
+ const struct sky81452_platform_data *pdata)
+{
+ struct mfd_cell cells[] = {
+ {
+
