Re: [PATCH v2 2/5] MFD: RK808: Add new mfd driver for RK808
On Tue, 26 Aug 2014, Dmitry Torokhov wrote: > On Tue, Aug 26, 2014 at 10:22:05AM +0100, Lee Jones wrote: > > On Mon, 25 Aug 2014, Chris Zhong wrote: > > > On 08/20/2014 05:21 PM, Lee Jones wrote: > > > >On Wed, 20 Aug 2014, Chris Zhong wrote: > > > > > > > >>The RK808 chip is a power management IC for multimedia and handheld > > > >>devices. It contains the following components: > > > >> > > > >>- Regulators > > > >>- RTC > > > >> > > > >>The rk808 core driver is registered as a platform driver and provides > > > >>communication through I2C with the host device for the different > > > >>components. > > > >> > > > >>Signed-off-by: Chris Zhong > > > >>--- > > > > [...] > > > > > >>+ rk808->pdata = pdata; > > > >Remove pdata from 'struct rk808'. You can obtain it from dev. > > > > > > Can I keep this pdata in rk808, because it is used in the regulator > > > driver. > > > The one obtain from dev maybe empty. > > > > If the one in dev is empty, you should populate that instead. > > No, drivers should not populate platform data in devices - they do not > own it (unlike drvdata). Platform data should be read-only so that if > one would unbind and then try to re-bind the driver we'd end up in > exactly same state as before. Right. I guess this point is moot now, as my other point about pdata not actually being required has been accecpted. But, when I say "you can obtain it from dev" I meant via dev_get_platdata(dev), rather than dev_get_platdata(dev->parent). So if this 'pdata' has to go somewhere, rather than sticking it in the MFD's (parent) platform_data, the .platform_data attribute in 'struct mfd_cell' should be used. > For DT systems we should be allocating platform data separately and make > sure we clean up after ourselves. -- 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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
On Tue, 26 Aug 2014, Dmitry Torokhov wrote: On Tue, Aug 26, 2014 at 10:22:05AM +0100, Lee Jones wrote: On Mon, 25 Aug 2014, Chris Zhong wrote: On 08/20/2014 05:21 PM, Lee Jones wrote: On Wed, 20 Aug 2014, Chris Zhong wrote: The RK808 chip is a power management IC for multimedia and handheld devices. It contains the following components: - Regulators - RTC The rk808 core driver is registered as a platform driver and provides communication through I2C with the host device for the different components. Signed-off-by: Chris Zhong z...@rock-chips.com --- [...] + rk808-pdata = pdata; Remove pdata from 'struct rk808'. You can obtain it from dev. Can I keep this pdata in rk808, because it is used in the regulator driver. The one obtain from dev maybe empty. If the one in dev is empty, you should populate that instead. No, drivers should not populate platform data in devices - they do not own it (unlike drvdata). Platform data should be read-only so that if one would unbind and then try to re-bind the driver we'd end up in exactly same state as before. Right. I guess this point is moot now, as my other point about pdata not actually being required has been accecpted. But, when I say you can obtain it from dev I meant via dev_get_platdata(dev), rather than dev_get_platdata(dev-parent). So if this 'pdata' has to go somewhere, rather than sticking it in the MFD's (parent) platform_data, the .platform_data attribute in 'struct mfd_cell' should be used. For DT systems we should be allocating platform data separately and make sure we clean up after ourselves. -- 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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
Hi, On Tue, Aug 26, 2014 at 9:28 AM, Dmitry Torokhov wrote: > Hi Lee, > > On Tue, Aug 26, 2014 at 10:22:05AM +0100, Lee Jones wrote: >> On Mon, 25 Aug 2014, Chris Zhong wrote: >> > On 08/20/2014 05:21 PM, Lee Jones wrote: >> > >On Wed, 20 Aug 2014, Chris Zhong wrote: >> > > >> > >>The RK808 chip is a power management IC for multimedia and handheld >> > >>devices. It contains the following components: >> > >> >> > >>- Regulators >> > >>- RTC >> > >> >> > >>The rk808 core driver is registered as a platform driver and provides >> > >>communication through I2C with the host device for the different >> > >>components. >> > >> >> > >>Signed-off-by: Chris Zhong >> > >>--- >> >> [...] >> >> > >>+ rk808->pdata = pdata; >> > >Remove pdata from 'struct rk808'. You can obtain it from dev. >> > >> > Can I keep this pdata in rk808, because it is used in the regulator driver. >> > The one obtain from dev maybe empty. >> >> If the one in dev is empty, you should populate that instead. > > No, drivers should not populate platform data in devices - they do not > own it (unlike drvdata). Platform data should be read-only so that if > one would unbind and then try to re-bind the driver we'd end up in > exactly same state as before. > > For DT systems we should be allocating platform data separately and make > sure we clean up after ourselves. Given Dmitry's advice it seems like Chris's old code (allocate pdata and store it in the driver's private structures) was correct. That also seems to match what I've seen other drivers do. Of course what Chris ended up doing was basically saying that "device tree" is required for rk808 and that you can't pass data into the driver using pdata (at least in v6 you can't specify "rockchip,system-power-controller" unless you're using device tree). To me that doesn't seem terrible. ...though he should probably finish what he started by: * Moving "struct rk808_board" so it's private to the regulator .c file * Don't even pretend you can get stuff from dev_get_platdata() in rk808-regulator.c * Add a dependency on "OF" in the Kconfig for rk808. -Doug -- 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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
Hi Lee, On Tue, Aug 26, 2014 at 10:22:05AM +0100, Lee Jones wrote: > On Mon, 25 Aug 2014, Chris Zhong wrote: > > On 08/20/2014 05:21 PM, Lee Jones wrote: > > >On Wed, 20 Aug 2014, Chris Zhong wrote: > > > > > >>The RK808 chip is a power management IC for multimedia and handheld > > >>devices. It contains the following components: > > >> > > >>- Regulators > > >>- RTC > > >> > > >>The rk808 core driver is registered as a platform driver and provides > > >>communication through I2C with the host device for the different > > >>components. > > >> > > >>Signed-off-by: Chris Zhong > > >>--- > > [...] > > > >>+ rk808->pdata = pdata; > > >Remove pdata from 'struct rk808'. You can obtain it from dev. > > > > Can I keep this pdata in rk808, because it is used in the regulator driver. > > The one obtain from dev maybe empty. > > If the one in dev is empty, you should populate that instead. No, drivers should not populate platform data in devices - they do not own it (unlike drvdata). Platform data should be read-only so that if one would unbind and then try to re-bind the driver we'd end up in exactly same state as before. For DT systems we should be allocating platform data separately and make sure we clean up after ourselves. Thanks. -- Dmitry -- 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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
On Tue, 26 Aug 2014, Chris Zhong wrote: [...] > Well, it might be better to delete the pdata from MFD. Bingo! That's what I've been trying to get at. -- 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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
On 08/26/2014 08:23 PM, Lee Jones wrote:
On Tue, 26 Aug 2014, Chris Zhong wrote:
On 08/26/2014 07:20 PM, Lee Jones wrote:
On Tue, 26 Aug 2014, Chris Zhong wrote:
On 08/26/2014 06:20 PM, Lee Jones wrote:
On Tue, 26 Aug 2014, Chris Zhong wrote:
On 08/26/2014 05:22 PM, Lee Jones wrote:
On Mon, 25 Aug 2014, Chris Zhong wrote:
On 08/20/2014 05:21 PM, Lee Jones wrote:
On Wed, 20 Aug 2014, Chris Zhong wrote:
The RK808 chip is a power management IC for multimedia and handheld
devices. It contains the following components:
- Regulators
- RTC
The rk808 core driver is registered as a platform driver and provides
communication through I2C with the host device for the different
components.
Signed-off-by: Chris Zhong
---
[...]
+ rk808->pdata = pdata;
Remove pdata from 'struct rk808'. You can obtain it from dev.
Can I keep this pdata in rk808, because it is used in the regulator driver.
The one obtain from dev maybe empty.
If the one in dev is empty, you should populate that instead.
So, should I malloc a pada, and assign it to client->dev?
If client->dev.pdata is NULL, yes.
But actually, I have more important questions that need to be answered
first. Ones which I would normally be able to answer myself if the
patch-set had been sent as one (i.e. threaded) instead of as
individual patches:
- What are you using pdata for?
For save some properties,
like "rockchip,system-power-controller" in MFD
and some regulator properties in regulator/rk808...
- Where is pdata populated?
It is populated in probe in mfd/rk808.c
actually, I copy it from tps65910.c
- Where is pdata used?
pdata is used in mfd and regulator
I'm still a little confused. I see it being populated in the MFD
driver, then I only see the attributes populated in the MFD driver
used in the MFD driver and nowhere else? I do see other attributes
used in the regulator driver i.e. .of_node[], but these are then used
only in the regulator driver, thus they shouldn't really be pdata.
Let me put it another way:
struct rk808_board {
+ int irq;
+ int irq_base;
+ int wakeup;
+ bool pm_off;
+ struct regulator_init_data *rk808_init_data[rk808_NUM_REGULATORS];
+ struct device_node *of_node[rk808_NUM_REGULATORS];
+ int pmic_sleep_gpio;
+ unsigned int dcdc_slp_voltage[3]; /* buckx_voltage in uV */
+ unsigned int ldo_slp_voltage[7];
+};
For each of the above:
- Can it be passed from platform data i.e. arch//{plat,mach}-*?
- Can it use local (not passed from driver to driver) variable instead?
If the answer to the first question is 'no' and/or if the answer to
the second question is 'yes', then it shouldn't be platform data.
Yes, if we have no dts file, these value can pass from mach-rk***.
It can be filled with all user setting of rk808, in a struct.
the latest struct is:
struct rk808_board {
int wakeup;
bool pm_off;
struct regulator_init_data *rk808_init_data[RK808_NUM_REGULATORS];
struct device_node *of_node[RK808_NUM_REGULATORS];
unsigned int dcdc_slp_voltage[3]; /* buckx_voltage in uV */
unsigned int ldo_slp_voltage[7];
};
This version looks much more acceptable.
Although, I am concerned about of_node[]. Isn't that only populated
and used in the regulator driver?
Yes, it only used in regulator driver
Well, it might be better to delete the pdata from MFD.
--
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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
On Tue, 26 Aug 2014, Chris Zhong wrote:
> On 08/26/2014 07:20 PM, Lee Jones wrote:
> >On Tue, 26 Aug 2014, Chris Zhong wrote:
> >>On 08/26/2014 06:20 PM, Lee Jones wrote:
> >>>On Tue, 26 Aug 2014, Chris Zhong wrote:
> On 08/26/2014 05:22 PM, Lee Jones wrote:
> >On Mon, 25 Aug 2014, Chris Zhong wrote:
> >>On 08/20/2014 05:21 PM, Lee Jones wrote:
> >>>On Wed, 20 Aug 2014, Chris Zhong wrote:
> >>>
> The RK808 chip is a power management IC for multimedia and handheld
> devices. It contains the following components:
>
> - Regulators
> - RTC
>
> The rk808 core driver is registered as a platform driver and provides
> communication through I2C with the host device for the different
> components.
>
> Signed-off-by: Chris Zhong
> ---
> >[...]
> >
> + rk808->pdata = pdata;
> >>>Remove pdata from 'struct rk808'. You can obtain it from dev.
> >>Can I keep this pdata in rk808, because it is used in the regulator
> >>driver.
> >>The one obtain from dev maybe empty.
> >If the one in dev is empty, you should populate that instead.
> So, should I malloc a pada, and assign it to client->dev?
> >>>If client->dev.pdata is NULL, yes.
> >>>
> >>>But actually, I have more important questions that need to be answered
> >>>first. Ones which I would normally be able to answer myself if the
> >>>patch-set had been sent as one (i.e. threaded) instead of as
> >>>individual patches:
> >>>
> >>>- What are you using pdata for?
> >>For save some properties,
> >>
> >>like "rockchip,system-power-controller" in MFD
> >>and some regulator properties in regulator/rk808...
> >>
> >>>- Where is pdata populated?
> >>It is populated in probe in mfd/rk808.c
> >>
> >>actually, I copy it from tps65910.c
> >>
> >>
> >>>- Where is pdata used?
> >>pdata is used in mfd and regulator
> >I'm still a little confused. I see it being populated in the MFD
> >driver, then I only see the attributes populated in the MFD driver
> >used in the MFD driver and nowhere else? I do see other attributes
> >used in the regulator driver i.e. .of_node[], but these are then used
> >only in the regulator driver, thus they shouldn't really be pdata.
> >
> >Let me put it another way:
> >
> >struct rk808_board {
> >+ int irq;
> >+ int irq_base;
> >+ int wakeup;
> >+ bool pm_off;
> >+ struct regulator_init_data *rk808_init_data[rk808_NUM_REGULATORS];
> >+ struct device_node *of_node[rk808_NUM_REGULATORS];
> >+ int pmic_sleep_gpio;
> >+ unsigned int dcdc_slp_voltage[3]; /* buckx_voltage in uV */
> >+ unsigned int ldo_slp_voltage[7];
> >+};
> >
> >For each of the above:
> >
> >- Can it be passed from platform data i.e. arch//{plat,mach}-*?
> >- Can it use local (not passed from driver to driver) variable instead?
> >
> >If the answer to the first question is 'no' and/or if the answer to
> >the second question is 'yes', then it shouldn't be platform data.
>
> Yes, if we have no dts file, these value can pass from mach-rk***.
>
> It can be filled with all user setting of rk808, in a struct.
>
> the latest struct is:
>
> struct rk808_board {
>
> int wakeup;
>
> bool pm_off;
>
> struct regulator_init_data *rk808_init_data[RK808_NUM_REGULATORS];
>
> struct device_node *of_node[RK808_NUM_REGULATORS];
>
> unsigned int dcdc_slp_voltage[3]; /* buckx_voltage in uV */
>
> unsigned int ldo_slp_voltage[7];
>
> };
This version looks much more acceptable.
Although, I am concerned about of_node[]. Isn't that only populated
and used in the regulator driver?
--
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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
On 08/26/2014 07:20 PM, Lee Jones wrote:
On Tue, 26 Aug 2014, Chris Zhong wrote:
On 08/26/2014 06:20 PM, Lee Jones wrote:
On Tue, 26 Aug 2014, Chris Zhong wrote:
On 08/26/2014 05:22 PM, Lee Jones wrote:
On Mon, 25 Aug 2014, Chris Zhong wrote:
On 08/20/2014 05:21 PM, Lee Jones wrote:
On Wed, 20 Aug 2014, Chris Zhong wrote:
The RK808 chip is a power management IC for multimedia and handheld
devices. It contains the following components:
- Regulators
- RTC
The rk808 core driver is registered as a platform driver and provides
communication through I2C with the host device for the different
components.
Signed-off-by: Chris Zhong
---
[...]
+ rk808->pdata = pdata;
Remove pdata from 'struct rk808'. You can obtain it from dev.
Can I keep this pdata in rk808, because it is used in the regulator driver.
The one obtain from dev maybe empty.
If the one in dev is empty, you should populate that instead.
So, should I malloc a pada, and assign it to client->dev?
If client->dev.pdata is NULL, yes.
But actually, I have more important questions that need to be answered
first. Ones which I would normally be able to answer myself if the
patch-set had been sent as one (i.e. threaded) instead of as
individual patches:
- What are you using pdata for?
For save some properties,
like "rockchip,system-power-controller" in MFD
and some regulator properties in regulator/rk808...
- Where is pdata populated?
It is populated in probe in mfd/rk808.c
actually, I copy it from tps65910.c
- Where is pdata used?
pdata is used in mfd and regulator
I'm still a little confused. I see it being populated in the MFD
driver, then I only see the attributes populated in the MFD driver
used in the MFD driver and nowhere else? I do see other attributes
used in the regulator driver i.e. .of_node[], but these are then used
only in the regulator driver, thus they shouldn't really be pdata.
Let me put it another way:
struct rk808_board {
+ int irq;
+ int irq_base;
+ int wakeup;
+ bool pm_off;
+ struct regulator_init_data *rk808_init_data[rk808_NUM_REGULATORS];
+ struct device_node *of_node[rk808_NUM_REGULATORS];
+ int pmic_sleep_gpio;
+ unsigned int dcdc_slp_voltage[3]; /* buckx_voltage in uV */
+ unsigned int ldo_slp_voltage[7];
+};
For each of the above:
- Can it be passed from platform data i.e. arch//{plat,mach}-*?
- Can it use local (not passed from driver to driver) variable instead?
If the answer to the first question is 'no' and/or if the answer to
the second question is 'yes', then it shouldn't be platform data.
Yes, if we have no dts file, these value can pass from mach-rk***.
It can be filled with all user setting of rk808, in a struct.
the latest struct is:
struct rk808_board {
int wakeup;
bool pm_off;
struct regulator_init_data *rk808_init_data[RK808_NUM_REGULATORS];
struct device_node *of_node[RK808_NUM_REGULATORS];
unsigned int dcdc_slp_voltage[3]; /* buckx_voltage in uV */
unsigned int ldo_slp_voltage[7];
};
--
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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
On Tue, 26 Aug 2014, Chris Zhong wrote:
> On 08/26/2014 06:20 PM, Lee Jones wrote:
> >On Tue, 26 Aug 2014, Chris Zhong wrote:
> >>On 08/26/2014 05:22 PM, Lee Jones wrote:
> >>>On Mon, 25 Aug 2014, Chris Zhong wrote:
> On 08/20/2014 05:21 PM, Lee Jones wrote:
> >On Wed, 20 Aug 2014, Chris Zhong wrote:
> >
> >>The RK808 chip is a power management IC for multimedia and handheld
> >>devices. It contains the following components:
> >>
> >>- Regulators
> >>- RTC
> >>
> >>The rk808 core driver is registered as a platform driver and provides
> >>communication through I2C with the host device for the different
> >>components.
> >>
> >>Signed-off-by: Chris Zhong
> >>---
> >>>[...]
> >>>
> >>+ rk808->pdata = pdata;
> >Remove pdata from 'struct rk808'. You can obtain it from dev.
> Can I keep this pdata in rk808, because it is used in the regulator
> driver.
> The one obtain from dev maybe empty.
> >>>If the one in dev is empty, you should populate that instead.
> >>So, should I malloc a pada, and assign it to client->dev?
> >If client->dev.pdata is NULL, yes.
> >
> >But actually, I have more important questions that need to be answered
> >first. Ones which I would normally be able to answer myself if the
> >patch-set had been sent as one (i.e. threaded) instead of as
> >individual patches:
> >
> >- What are you using pdata for?
>
> For save some properties,
>
> like "rockchip,system-power-controller" in MFD
> and some regulator properties in regulator/rk808...
>
> >- Where is pdata populated?
>
> It is populated in probe in mfd/rk808.c
>
> actually, I copy it from tps65910.c
>
>
> >- Where is pdata used?
>
> pdata is used in mfd and regulator
I'm still a little confused. I see it being populated in the MFD
driver, then I only see the attributes populated in the MFD driver
used in the MFD driver and nowhere else? I do see other attributes
used in the regulator driver i.e. .of_node[], but these are then used
only in the regulator driver, thus they shouldn't really be pdata.
Let me put it another way:
struct rk808_board {
+ int irq;
+ int irq_base;
+ int wakeup;
+ bool pm_off;
+ struct regulator_init_data *rk808_init_data[rk808_NUM_REGULATORS];
+ struct device_node *of_node[rk808_NUM_REGULATORS];
+ int pmic_sleep_gpio;
+ unsigned int dcdc_slp_voltage[3]; /* buckx_voltage in uV */
+ unsigned int ldo_slp_voltage[7];
+};
For each of the above:
- Can it be passed from platform data i.e. arch//{plat,mach}-*?
- Can it use local (not passed from driver to driver) variable instead?
If the answer to the first question is 'no' and/or if the answer to
the second question is 'yes', then it shouldn't be platform data.
--
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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
On 08/26/2014 06:20 PM, Lee Jones wrote: On Tue, 26 Aug 2014, Chris Zhong wrote: On 08/26/2014 05:22 PM, Lee Jones wrote: On Mon, 25 Aug 2014, Chris Zhong wrote: On 08/20/2014 05:21 PM, Lee Jones wrote: On Wed, 20 Aug 2014, Chris Zhong wrote: The RK808 chip is a power management IC for multimedia and handheld devices. It contains the following components: - Regulators - RTC The rk808 core driver is registered as a platform driver and provides communication through I2C with the host device for the different components. Signed-off-by: Chris Zhong --- [...] + rk808->pdata = pdata; Remove pdata from 'struct rk808'. You can obtain it from dev. Can I keep this pdata in rk808, because it is used in the regulator driver. The one obtain from dev maybe empty. If the one in dev is empty, you should populate that instead. So, should I malloc a pada, and assign it to client->dev? If client->dev.pdata is NULL, yes. But actually, I have more important questions that need to be answered first. Ones which I would normally be able to answer myself if the patch-set had been sent as one (i.e. threaded) instead of as individual patches: - What are you using pdata for? For save some properties, like "rockchip,system-power-controller" in MFD and some regulator properties in regulator/rk808... - Where is pdata populated? It is populated in probe in mfd/rk808.c actually, I copy it from tps65910.c - Where is pdata used? pdata is used in mfd and regulator -- 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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
On Tue, 26 Aug 2014, Chris Zhong wrote: > On 08/26/2014 05:22 PM, Lee Jones wrote: > >On Mon, 25 Aug 2014, Chris Zhong wrote: > >>On 08/20/2014 05:21 PM, Lee Jones wrote: > >>>On Wed, 20 Aug 2014, Chris Zhong wrote: > >>> > The RK808 chip is a power management IC for multimedia and handheld > devices. It contains the following components: > > - Regulators > - RTC > > The rk808 core driver is registered as a platform driver and provides > communication through I2C with the host device for the different > components. > > Signed-off-by: Chris Zhong > --- > >[...] > > > + rk808->pdata = pdata; > >>>Remove pdata from 'struct rk808'. You can obtain it from dev. > >>Can I keep this pdata in rk808, because it is used in the regulator driver. > >>The one obtain from dev maybe empty. > >If the one in dev is empty, you should populate that instead. > > So, should I malloc a pada, and assign it to client->dev? If client->dev.pdata is NULL, yes. But actually, I have more important questions that need to be answered first. Ones which I would normally be able to answer myself if the patch-set had been sent as one (i.e. threaded) instead of as individual patches: - What are you using pdata for? - Where is pdata populated? - Where is pdata used? -- 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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
On 08/26/2014 05:22 PM, Lee Jones wrote: On Mon, 25 Aug 2014, Chris Zhong wrote: On 08/20/2014 05:21 PM, Lee Jones wrote: On Wed, 20 Aug 2014, Chris Zhong wrote: The RK808 chip is a power management IC for multimedia and handheld devices. It contains the following components: - Regulators - RTC The rk808 core driver is registered as a platform driver and provides communication through I2C with the host device for the different components. Signed-off-by: Chris Zhong --- [...] + rk808->pdata = pdata; Remove pdata from 'struct rk808'. You can obtain it from dev. Can I keep this pdata in rk808, because it is used in the regulator driver. The one obtain from dev maybe empty. If the one in dev is empty, you should populate that instead. So, should I malloc a pada, and assign it to client->dev? -- 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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
On Mon, 25 Aug 2014, Chris Zhong wrote: > On 08/20/2014 05:21 PM, Lee Jones wrote: > >On Wed, 20 Aug 2014, Chris Zhong wrote: > > > >>The RK808 chip is a power management IC for multimedia and handheld > >>devices. It contains the following components: > >> > >>- Regulators > >>- RTC > >> > >>The rk808 core driver is registered as a platform driver and provides > >>communication through I2C with the host device for the different > >>components. > >> > >>Signed-off-by: Chris Zhong > >>--- [...] > >>+ rk808->pdata = pdata; > >Remove pdata from 'struct rk808'. You can obtain it from dev. > > Can I keep this pdata in rk808, because it is used in the regulator driver. > The one obtain from dev maybe empty. If the one in dev is empty, you should populate that instead. [...] -- 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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
On Mon, 25 Aug 2014, Chris Zhong wrote: On 08/20/2014 05:21 PM, Lee Jones wrote: On Wed, 20 Aug 2014, Chris Zhong wrote: The RK808 chip is a power management IC for multimedia and handheld devices. It contains the following components: - Regulators - RTC The rk808 core driver is registered as a platform driver and provides communication through I2C with the host device for the different components. Signed-off-by: Chris Zhong z...@rock-chips.com --- [...] + rk808-pdata = pdata; Remove pdata from 'struct rk808'. You can obtain it from dev. Can I keep this pdata in rk808, because it is used in the regulator driver. The one obtain from dev maybe empty. If the one in dev is empty, you should populate that instead. [...] -- 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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
On 08/26/2014 05:22 PM, Lee Jones wrote: On Mon, 25 Aug 2014, Chris Zhong wrote: On 08/20/2014 05:21 PM, Lee Jones wrote: On Wed, 20 Aug 2014, Chris Zhong wrote: The RK808 chip is a power management IC for multimedia and handheld devices. It contains the following components: - Regulators - RTC The rk808 core driver is registered as a platform driver and provides communication through I2C with the host device for the different components. Signed-off-by: Chris Zhong z...@rock-chips.com --- [...] + rk808-pdata = pdata; Remove pdata from 'struct rk808'. You can obtain it from dev. Can I keep this pdata in rk808, because it is used in the regulator driver. The one obtain from dev maybe empty. If the one in dev is empty, you should populate that instead. So, should I malloc a pada, and assign it to client-dev? -- 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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
On Tue, 26 Aug 2014, Chris Zhong wrote: On 08/26/2014 05:22 PM, Lee Jones wrote: On Mon, 25 Aug 2014, Chris Zhong wrote: On 08/20/2014 05:21 PM, Lee Jones wrote: On Wed, 20 Aug 2014, Chris Zhong wrote: The RK808 chip is a power management IC for multimedia and handheld devices. It contains the following components: - Regulators - RTC The rk808 core driver is registered as a platform driver and provides communication through I2C with the host device for the different components. Signed-off-by: Chris Zhong z...@rock-chips.com --- [...] + rk808-pdata = pdata; Remove pdata from 'struct rk808'. You can obtain it from dev. Can I keep this pdata in rk808, because it is used in the regulator driver. The one obtain from dev maybe empty. If the one in dev is empty, you should populate that instead. So, should I malloc a pada, and assign it to client-dev? If client-dev.pdata is NULL, yes. But actually, I have more important questions that need to be answered first. Ones which I would normally be able to answer myself if the patch-set had been sent as one (i.e. threaded) instead of as individual patches: - What are you using pdata for? - Where is pdata populated? - Where is pdata used? -- 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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
On 08/26/2014 06:20 PM, Lee Jones wrote: On Tue, 26 Aug 2014, Chris Zhong wrote: On 08/26/2014 05:22 PM, Lee Jones wrote: On Mon, 25 Aug 2014, Chris Zhong wrote: On 08/20/2014 05:21 PM, Lee Jones wrote: On Wed, 20 Aug 2014, Chris Zhong wrote: The RK808 chip is a power management IC for multimedia and handheld devices. It contains the following components: - Regulators - RTC The rk808 core driver is registered as a platform driver and provides communication through I2C with the host device for the different components. Signed-off-by: Chris Zhong z...@rock-chips.com --- [...] + rk808-pdata = pdata; Remove pdata from 'struct rk808'. You can obtain it from dev. Can I keep this pdata in rk808, because it is used in the regulator driver. The one obtain from dev maybe empty. If the one in dev is empty, you should populate that instead. So, should I malloc a pada, and assign it to client-dev? If client-dev.pdata is NULL, yes. But actually, I have more important questions that need to be answered first. Ones which I would normally be able to answer myself if the patch-set had been sent as one (i.e. threaded) instead of as individual patches: - What are you using pdata for? For save some properties, like rockchip,system-power-controller in MFD and some regulator properties in regulator/rk808... - Where is pdata populated? It is populated in probe in mfd/rk808.c actually, I copy it from tps65910.c - Where is pdata used? pdata is used in mfd and regulator -- 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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
On Tue, 26 Aug 2014, Chris Zhong wrote:
On 08/26/2014 06:20 PM, Lee Jones wrote:
On Tue, 26 Aug 2014, Chris Zhong wrote:
On 08/26/2014 05:22 PM, Lee Jones wrote:
On Mon, 25 Aug 2014, Chris Zhong wrote:
On 08/20/2014 05:21 PM, Lee Jones wrote:
On Wed, 20 Aug 2014, Chris Zhong wrote:
The RK808 chip is a power management IC for multimedia and handheld
devices. It contains the following components:
- Regulators
- RTC
The rk808 core driver is registered as a platform driver and provides
communication through I2C with the host device for the different
components.
Signed-off-by: Chris Zhong z...@rock-chips.com
---
[...]
+ rk808-pdata = pdata;
Remove pdata from 'struct rk808'. You can obtain it from dev.
Can I keep this pdata in rk808, because it is used in the regulator
driver.
The one obtain from dev maybe empty.
If the one in dev is empty, you should populate that instead.
So, should I malloc a pada, and assign it to client-dev?
If client-dev.pdata is NULL, yes.
But actually, I have more important questions that need to be answered
first. Ones which I would normally be able to answer myself if the
patch-set had been sent as one (i.e. threaded) instead of as
individual patches:
- What are you using pdata for?
For save some properties,
like rockchip,system-power-controller in MFD
and some regulator properties in regulator/rk808...
- Where is pdata populated?
It is populated in probe in mfd/rk808.c
actually, I copy it from tps65910.c
- Where is pdata used?
pdata is used in mfd and regulator
I'm still a little confused. I see it being populated in the MFD
driver, then I only see the attributes populated in the MFD driver
used in the MFD driver and nowhere else? I do see other attributes
used in the regulator driver i.e. .of_node[], but these are then used
only in the regulator driver, thus they shouldn't really be pdata.
Let me put it another way:
struct rk808_board {
+ int irq;
+ int irq_base;
+ int wakeup;
+ bool pm_off;
+ struct regulator_init_data *rk808_init_data[rk808_NUM_REGULATORS];
+ struct device_node *of_node[rk808_NUM_REGULATORS];
+ int pmic_sleep_gpio;
+ unsigned int dcdc_slp_voltage[3]; /* buckx_voltage in uV */
+ unsigned int ldo_slp_voltage[7];
+};
For each of the above:
- Can it be passed from platform data i.e. arch/arch/{plat,mach}-*?
- Can it use local (not passed from driver to driver) variable instead?
If the answer to the first question is 'no' and/or if the answer to
the second question is 'yes', then it shouldn't be platform data.
--
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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
On 08/26/2014 07:20 PM, Lee Jones wrote:
On Tue, 26 Aug 2014, Chris Zhong wrote:
On 08/26/2014 06:20 PM, Lee Jones wrote:
On Tue, 26 Aug 2014, Chris Zhong wrote:
On 08/26/2014 05:22 PM, Lee Jones wrote:
On Mon, 25 Aug 2014, Chris Zhong wrote:
On 08/20/2014 05:21 PM, Lee Jones wrote:
On Wed, 20 Aug 2014, Chris Zhong wrote:
The RK808 chip is a power management IC for multimedia and handheld
devices. It contains the following components:
- Regulators
- RTC
The rk808 core driver is registered as a platform driver and provides
communication through I2C with the host device for the different
components.
Signed-off-by: Chris Zhong z...@rock-chips.com
---
[...]
+ rk808-pdata = pdata;
Remove pdata from 'struct rk808'. You can obtain it from dev.
Can I keep this pdata in rk808, because it is used in the regulator driver.
The one obtain from dev maybe empty.
If the one in dev is empty, you should populate that instead.
So, should I malloc a pada, and assign it to client-dev?
If client-dev.pdata is NULL, yes.
But actually, I have more important questions that need to be answered
first. Ones which I would normally be able to answer myself if the
patch-set had been sent as one (i.e. threaded) instead of as
individual patches:
- What are you using pdata for?
For save some properties,
like rockchip,system-power-controller in MFD
and some regulator properties in regulator/rk808...
- Where is pdata populated?
It is populated in probe in mfd/rk808.c
actually, I copy it from tps65910.c
- Where is pdata used?
pdata is used in mfd and regulator
I'm still a little confused. I see it being populated in the MFD
driver, then I only see the attributes populated in the MFD driver
used in the MFD driver and nowhere else? I do see other attributes
used in the regulator driver i.e. .of_node[], but these are then used
only in the regulator driver, thus they shouldn't really be pdata.
Let me put it another way:
struct rk808_board {
+ int irq;
+ int irq_base;
+ int wakeup;
+ bool pm_off;
+ struct regulator_init_data *rk808_init_data[rk808_NUM_REGULATORS];
+ struct device_node *of_node[rk808_NUM_REGULATORS];
+ int pmic_sleep_gpio;
+ unsigned int dcdc_slp_voltage[3]; /* buckx_voltage in uV */
+ unsigned int ldo_slp_voltage[7];
+};
For each of the above:
- Can it be passed from platform data i.e. arch/arch/{plat,mach}-*?
- Can it use local (not passed from driver to driver) variable instead?
If the answer to the first question is 'no' and/or if the answer to
the second question is 'yes', then it shouldn't be platform data.
Yes, if we have no dts file, these value can pass from mach-rk***.
It can be filled with all user setting of rk808, in a struct.
the latest struct is:
struct rk808_board {
int wakeup;
bool pm_off;
struct regulator_init_data *rk808_init_data[RK808_NUM_REGULATORS];
struct device_node *of_node[RK808_NUM_REGULATORS];
unsigned int dcdc_slp_voltage[3]; /* buckx_voltage in uV */
unsigned int ldo_slp_voltage[7];
};
--
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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
On Tue, 26 Aug 2014, Chris Zhong wrote:
On 08/26/2014 07:20 PM, Lee Jones wrote:
On Tue, 26 Aug 2014, Chris Zhong wrote:
On 08/26/2014 06:20 PM, Lee Jones wrote:
On Tue, 26 Aug 2014, Chris Zhong wrote:
On 08/26/2014 05:22 PM, Lee Jones wrote:
On Mon, 25 Aug 2014, Chris Zhong wrote:
On 08/20/2014 05:21 PM, Lee Jones wrote:
On Wed, 20 Aug 2014, Chris Zhong wrote:
The RK808 chip is a power management IC for multimedia and handheld
devices. It contains the following components:
- Regulators
- RTC
The rk808 core driver is registered as a platform driver and provides
communication through I2C with the host device for the different
components.
Signed-off-by: Chris Zhong z...@rock-chips.com
---
[...]
+ rk808-pdata = pdata;
Remove pdata from 'struct rk808'. You can obtain it from dev.
Can I keep this pdata in rk808, because it is used in the regulator
driver.
The one obtain from dev maybe empty.
If the one in dev is empty, you should populate that instead.
So, should I malloc a pada, and assign it to client-dev?
If client-dev.pdata is NULL, yes.
But actually, I have more important questions that need to be answered
first. Ones which I would normally be able to answer myself if the
patch-set had been sent as one (i.e. threaded) instead of as
individual patches:
- What are you using pdata for?
For save some properties,
like rockchip,system-power-controller in MFD
and some regulator properties in regulator/rk808...
- Where is pdata populated?
It is populated in probe in mfd/rk808.c
actually, I copy it from tps65910.c
- Where is pdata used?
pdata is used in mfd and regulator
I'm still a little confused. I see it being populated in the MFD
driver, then I only see the attributes populated in the MFD driver
used in the MFD driver and nowhere else? I do see other attributes
used in the regulator driver i.e. .of_node[], but these are then used
only in the regulator driver, thus they shouldn't really be pdata.
Let me put it another way:
struct rk808_board {
+ int irq;
+ int irq_base;
+ int wakeup;
+ bool pm_off;
+ struct regulator_init_data *rk808_init_data[rk808_NUM_REGULATORS];
+ struct device_node *of_node[rk808_NUM_REGULATORS];
+ int pmic_sleep_gpio;
+ unsigned int dcdc_slp_voltage[3]; /* buckx_voltage in uV */
+ unsigned int ldo_slp_voltage[7];
+};
For each of the above:
- Can it be passed from platform data i.e. arch/arch/{plat,mach}-*?
- Can it use local (not passed from driver to driver) variable instead?
If the answer to the first question is 'no' and/or if the answer to
the second question is 'yes', then it shouldn't be platform data.
Yes, if we have no dts file, these value can pass from mach-rk***.
It can be filled with all user setting of rk808, in a struct.
the latest struct is:
struct rk808_board {
int wakeup;
bool pm_off;
struct regulator_init_data *rk808_init_data[RK808_NUM_REGULATORS];
struct device_node *of_node[RK808_NUM_REGULATORS];
unsigned int dcdc_slp_voltage[3]; /* buckx_voltage in uV */
unsigned int ldo_slp_voltage[7];
};
This version looks much more acceptable.
Although, I am concerned about of_node[]. Isn't that only populated
and used in the regulator driver?
--
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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
On 08/26/2014 08:23 PM, Lee Jones wrote:
On Tue, 26 Aug 2014, Chris Zhong wrote:
On 08/26/2014 07:20 PM, Lee Jones wrote:
On Tue, 26 Aug 2014, Chris Zhong wrote:
On 08/26/2014 06:20 PM, Lee Jones wrote:
On Tue, 26 Aug 2014, Chris Zhong wrote:
On 08/26/2014 05:22 PM, Lee Jones wrote:
On Mon, 25 Aug 2014, Chris Zhong wrote:
On 08/20/2014 05:21 PM, Lee Jones wrote:
On Wed, 20 Aug 2014, Chris Zhong wrote:
The RK808 chip is a power management IC for multimedia and handheld
devices. It contains the following components:
- Regulators
- RTC
The rk808 core driver is registered as a platform driver and provides
communication through I2C with the host device for the different
components.
Signed-off-by: Chris Zhong z...@rock-chips.com
---
[...]
+ rk808-pdata = pdata;
Remove pdata from 'struct rk808'. You can obtain it from dev.
Can I keep this pdata in rk808, because it is used in the regulator driver.
The one obtain from dev maybe empty.
If the one in dev is empty, you should populate that instead.
So, should I malloc a pada, and assign it to client-dev?
If client-dev.pdata is NULL, yes.
But actually, I have more important questions that need to be answered
first. Ones which I would normally be able to answer myself if the
patch-set had been sent as one (i.e. threaded) instead of as
individual patches:
- What are you using pdata for?
For save some properties,
like rockchip,system-power-controller in MFD
and some regulator properties in regulator/rk808...
- Where is pdata populated?
It is populated in probe in mfd/rk808.c
actually, I copy it from tps65910.c
- Where is pdata used?
pdata is used in mfd and regulator
I'm still a little confused. I see it being populated in the MFD
driver, then I only see the attributes populated in the MFD driver
used in the MFD driver and nowhere else? I do see other attributes
used in the regulator driver i.e. .of_node[], but these are then used
only in the regulator driver, thus they shouldn't really be pdata.
Let me put it another way:
struct rk808_board {
+ int irq;
+ int irq_base;
+ int wakeup;
+ bool pm_off;
+ struct regulator_init_data *rk808_init_data[rk808_NUM_REGULATORS];
+ struct device_node *of_node[rk808_NUM_REGULATORS];
+ int pmic_sleep_gpio;
+ unsigned int dcdc_slp_voltage[3]; /* buckx_voltage in uV */
+ unsigned int ldo_slp_voltage[7];
+};
For each of the above:
- Can it be passed from platform data i.e. arch/arch/{plat,mach}-*?
- Can it use local (not passed from driver to driver) variable instead?
If the answer to the first question is 'no' and/or if the answer to
the second question is 'yes', then it shouldn't be platform data.
Yes, if we have no dts file, these value can pass from mach-rk***.
It can be filled with all user setting of rk808, in a struct.
the latest struct is:
struct rk808_board {
int wakeup;
bool pm_off;
struct regulator_init_data *rk808_init_data[RK808_NUM_REGULATORS];
struct device_node *of_node[RK808_NUM_REGULATORS];
unsigned int dcdc_slp_voltage[3]; /* buckx_voltage in uV */
unsigned int ldo_slp_voltage[7];
};
This version looks much more acceptable.
Although, I am concerned about of_node[]. Isn't that only populated
and used in the regulator driver?
Yes, it only used in regulator driver
Well, it might be better to delete the pdata from MFD.
--
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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
On Tue, 26 Aug 2014, Chris Zhong wrote: [...] Well, it might be better to delete the pdata from MFD. Bingo! That's what I've been trying to get at. -- 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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
Hi Lee, On Tue, Aug 26, 2014 at 10:22:05AM +0100, Lee Jones wrote: On Mon, 25 Aug 2014, Chris Zhong wrote: On 08/20/2014 05:21 PM, Lee Jones wrote: On Wed, 20 Aug 2014, Chris Zhong wrote: The RK808 chip is a power management IC for multimedia and handheld devices. It contains the following components: - Regulators - RTC The rk808 core driver is registered as a platform driver and provides communication through I2C with the host device for the different components. Signed-off-by: Chris Zhong z...@rock-chips.com --- [...] + rk808-pdata = pdata; Remove pdata from 'struct rk808'. You can obtain it from dev. Can I keep this pdata in rk808, because it is used in the regulator driver. The one obtain from dev maybe empty. If the one in dev is empty, you should populate that instead. No, drivers should not populate platform data in devices - they do not own it (unlike drvdata). Platform data should be read-only so that if one would unbind and then try to re-bind the driver we'd end up in exactly same state as before. For DT systems we should be allocating platform data separately and make sure we clean up after ourselves. Thanks. -- Dmitry -- 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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
Hi, On Tue, Aug 26, 2014 at 9:28 AM, Dmitry Torokhov dmitry.torok...@gmail.com wrote: Hi Lee, On Tue, Aug 26, 2014 at 10:22:05AM +0100, Lee Jones wrote: On Mon, 25 Aug 2014, Chris Zhong wrote: On 08/20/2014 05:21 PM, Lee Jones wrote: On Wed, 20 Aug 2014, Chris Zhong wrote: The RK808 chip is a power management IC for multimedia and handheld devices. It contains the following components: - Regulators - RTC The rk808 core driver is registered as a platform driver and provides communication through I2C with the host device for the different components. Signed-off-by: Chris Zhong z...@rock-chips.com --- [...] + rk808-pdata = pdata; Remove pdata from 'struct rk808'. You can obtain it from dev. Can I keep this pdata in rk808, because it is used in the regulator driver. The one obtain from dev maybe empty. If the one in dev is empty, you should populate that instead. No, drivers should not populate platform data in devices - they do not own it (unlike drvdata). Platform data should be read-only so that if one would unbind and then try to re-bind the driver we'd end up in exactly same state as before. For DT systems we should be allocating platform data separately and make sure we clean up after ourselves. Given Dmitry's advice it seems like Chris's old code (allocate pdata and store it in the driver's private structures) was correct. That also seems to match what I've seen other drivers do. Of course what Chris ended up doing was basically saying that device tree is required for rk808 and that you can't pass data into the driver using pdata (at least in v6 you can't specify rockchip,system-power-controller unless you're using device tree). To me that doesn't seem terrible. ...though he should probably finish what he started by: * Moving struct rk808_board so it's private to the regulator .c file * Don't even pretend you can get stuff from dev_get_platdata() in rk808-regulator.c * Add a dependency on OF in the Kconfig for rk808. -Doug -- 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 v2 2/5] MFD: RK808: Add new mfd driver for RK808
On 08/20/2014 05:21 PM, Lee Jones wrote:
On Wed, 20 Aug 2014, Chris Zhong wrote:
The RK808 chip is a power management IC for multimedia and handheld
devices. It contains the following components:
- Regulators
- RTC
The rk808 core driver is registered as a platform driver and provides
communication through I2C with the host device for the different
components.
Signed-off-by: Chris Zhong
---
Changes in v2:
Adviced by Mark Browm:
- change of_find_node_by_name to find_child_by_name
- use RK808_NUM_REGULATORS as the name of the constant
- create a pdata when missing platform data
- use the rk808_reg name to supply_regulator name
- replace regulator_register with devm_regulator_register
- some other problem with coding style
drivers/mfd/Kconfig | 13 ++
drivers/mfd/Makefile |1 +
drivers/mfd/rk808.c | 297 +
include/linux/mfd/rk808.h | 219 +
4 files changed, 530 insertions(+)
create mode 100644 drivers/mfd/rk808.c
create mode 100644 include/linux/mfd/rk808.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de5abf2..1df133e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -582,6 +582,19 @@ config MFD_RC5T583
Additional drivers must be enabled in order to use the
different functionality of the device.
+config MFD_RK808
+ tristate "Rockchip RK808 Power Management chip"
+ depends on I2C
+ select MFD_CORE
+ select REGMAP_I2C
+ select REGMAP_IRQ
+ help
<-- Use more of the allotted space
--->
+ Select this option to get support for the RK808 Power
+ Management system device.
What's a 'system device', and how does that differ to a controller?
+ This driver provides common support for accessing the device
+ through i2c interface. The device supports multiple sub-devices
s/i2c/I2C/
+ like interrupts, RTC, LDO and DCDC regulators, onkey.
s/like/including/
I would s/and/&/, then put an "and" before "onkey".
this, this, this 'and' that.
config MFD_SEC_CORE
bool "SAMSUNG Electronics PMIC Series Support"
depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f001487..dbc28e7 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -160,6 +160,7 @@ obj-$(CONFIG_MFD_INTEL_MSIC)+= intel_msic.o
obj-$(CONFIG_MFD_PALMAS) += palmas.o
obj-$(CONFIG_MFD_VIPERBOARD)+= viperboard.o
obj-$(CONFIG_MFD_RC5T583) += rc5t583.o rc5t583-irq.o
+obj-$(CONFIG_MFD_RK808)+= rk808.o
obj-$(CONFIG_MFD_SEC_CORE)+= sec-core.o sec-irq.o
obj-$(CONFIG_MFD_SYSCON) += syscon.o
obj-$(CONFIG_MFD_LM3533) += lm3533-core.o lm3533-ctrlbank.o
diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
new file mode 100644
index 000..667cfdf
--- /dev/null
+++ b/drivers/mfd/rk808.c
@@ -0,0 +1,297 @@
+/*
+ * Mfd core driver for Rockchip RK808
s/Mfd/MFD
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * Author: Chris Zhong
+ * Author: Zhang Qing
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
Remove this line.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
I'm pretty sure you don't need all of these includes.
Remove the ones you're not using.
+struct rk808_reg_data {
+ int addr;
+ int mask;
+ int value;
+};
+
+static struct rk808 *g_rk808;
Grim.
+static struct resource rtc_resources[] = {
+ {
+ .start = RK808_IRQ_RTC_ALARM,
+ .end= RK808_IRQ_RTC_ALARM,
+ .flags = IORESOURCE_IRQ,
+ }
+};
+
+static const struct mfd_cell rk808s[] = {
+ {
+ .name = "rk808-regulator",
+ },
+ {
+ .name = "rk808-rtc",
+ .num_resources = ARRAY_SIZE(rtc_resources),
+ .resources = _resources[0],
+ },
+ {
+ .name = "rk808-clkout",
+ },
Can you reorder these, with the single liners at the start and
actually make them one line, so:
{ .name = "rk808-clkout" },
{ .name = "rk808-regulator" },
{
.name = "rk808-rtc",
.num_resources = ARRAY_SIZE(rtc_resources),
.resources = _resources[0],
},
This also happens to be alphabetical.
+};
+
+static const struct
Re: [PATCH v2 2/5] MFD: RK808: Add new mfd driver for RK808
On 08/20/2014 05:21 PM, Lee Jones wrote:
On Wed, 20 Aug 2014, Chris Zhong wrote:
The RK808 chip is a power management IC for multimedia and handheld
devices. It contains the following components:
- Regulators
- RTC
The rk808 core driver is registered as a platform driver and provides
communication through I2C with the host device for the different
components.
Signed-off-by: Chris Zhong z...@rock-chips.com
---
Changes in v2:
Adviced by Mark Browm:
- change of_find_node_by_name to find_child_by_name
- use RK808_NUM_REGULATORS as the name of the constant
- create a pdata when missing platform data
- use the rk808_reg name to supply_regulator name
- replace regulator_register with devm_regulator_register
- some other problem with coding style
drivers/mfd/Kconfig | 13 ++
drivers/mfd/Makefile |1 +
drivers/mfd/rk808.c | 297 +
include/linux/mfd/rk808.h | 219 +
4 files changed, 530 insertions(+)
create mode 100644 drivers/mfd/rk808.c
create mode 100644 include/linux/mfd/rk808.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de5abf2..1df133e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -582,6 +582,19 @@ config MFD_RC5T583
Additional drivers must be enabled in order to use the
different functionality of the device.
+config MFD_RK808
+ tristate Rockchip RK808 Power Management chip
+ depends on I2C
+ select MFD_CORE
+ select REGMAP_I2C
+ select REGMAP_IRQ
+ help
-- Use more of the allotted space
---
+ Select this option to get support for the RK808 Power
+ Management system device.
What's a 'system device', and how does that differ to a controller?
+ This driver provides common support for accessing the device
+ through i2c interface. The device supports multiple sub-devices
s/i2c/I2C/
+ like interrupts, RTC, LDO and DCDC regulators, onkey.
s/like/including/
I would s/and//, then put an and before onkey.
this, this, this 'and' that.
config MFD_SEC_CORE
bool SAMSUNG Electronics PMIC Series Support
depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f001487..dbc28e7 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -160,6 +160,7 @@ obj-$(CONFIG_MFD_INTEL_MSIC)+= intel_msic.o
obj-$(CONFIG_MFD_PALMAS) += palmas.o
obj-$(CONFIG_MFD_VIPERBOARD)+= viperboard.o
obj-$(CONFIG_MFD_RC5T583) += rc5t583.o rc5t583-irq.o
+obj-$(CONFIG_MFD_RK808)+= rk808.o
obj-$(CONFIG_MFD_SEC_CORE)+= sec-core.o sec-irq.o
obj-$(CONFIG_MFD_SYSCON) += syscon.o
obj-$(CONFIG_MFD_LM3533) += lm3533-core.o lm3533-ctrlbank.o
diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
new file mode 100644
index 000..667cfdf
--- /dev/null
+++ b/drivers/mfd/rk808.c
@@ -0,0 +1,297 @@
+/*
+ * Mfd core driver for Rockchip RK808
s/Mfd/MFD
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * Author: Chris Zhong z...@rock-chips.com
+ * Author: Zhang Qing zhangq...@rock-chips.com
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
Remove this line.
+ */
+
+#include linux/bug.h
+#include linux/delay.h
+#include linux/err.h
+#include linux/i2c.h
+#include linux/kernel.h
+#include linux/interrupt.h
+#include linux/mfd/rk808.h
+#include linux/mfd/core.h
+#include linux/mutex.h
+#include linux/module.h
+#include linux/of_irq.h
+#include linux/of_gpio.h
+#include linux/of.h
+#include linux/of_device.h
+#include linux/regulator/driver.h
+#include linux/regulator/of_regulator.h
+#include linux/regulator/driver.h
+#include linux/regulator/machine.h
+#include linux/regmap.h
+#include linux/slab.h
I'm pretty sure you don't need all of these includes.
Remove the ones you're not using.
+struct rk808_reg_data {
+ int addr;
+ int mask;
+ int value;
+};
+
+static struct rk808 *g_rk808;
Grim.
+static struct resource rtc_resources[] = {
+ {
+ .start = RK808_IRQ_RTC_ALARM,
+ .end= RK808_IRQ_RTC_ALARM,
+ .flags = IORESOURCE_IRQ,
+ }
+};
+
+static const struct mfd_cell rk808s[] = {
+ {
+ .name = rk808-regulator,
+ },
+ {
+ .name = rk808-rtc,
+ .num_resources = ARRAY_SIZE(rtc_resources),
+ .resources = rtc_resources[0],
+ },
+ {
+ .name = rk808-clkout,
+
Re: [PATCH v2 2/5] MFD: RK808: Add new mfd driver for RK808
Chris,
On Tue, Aug 19, 2014 at 8:31 PM, Chris Zhong wrote:
> The RK808 chip is a power management IC for multimedia and handheld
> devices. It contains the following components:
>
> - Regulators
> - RTC
>
> The rk808 core driver is registered as a platform driver and provides
> communication through I2C with the host device for the different
> components.
>
> Signed-off-by: Chris Zhong
>
> ---
>
> Changes in v2:
> Adviced by Mark Browm:
> - change of_find_node_by_name to find_child_by_name
> - use RK808_NUM_REGULATORS as the name of the constant
> - create a pdata when missing platform data
> - use the rk808_reg name to supply_regulator name
> - replace regulator_register with devm_regulator_register
> - some other problem with coding style
>
> drivers/mfd/Kconfig | 13 ++
> drivers/mfd/Makefile |1 +
> drivers/mfd/rk808.c | 297
> +
> include/linux/mfd/rk808.h | 219 +
> 4 files changed, 530 insertions(+)
> create mode 100644 drivers/mfd/rk808.c
> create mode 100644 include/linux/mfd/rk808.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index de5abf2..1df133e 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -582,6 +582,19 @@ config MFD_RC5T583
> Additional drivers must be enabled in order to use the
> different functionality of the device.
>
> +config MFD_RK808
> + tristate "Rockchip RK808 Power Management chip"
> + depends on I2C
> + select MFD_CORE
> + select REGMAP_I2C
> + select REGMAP_IRQ
> + help
> + Select this option to get support for the RK808 Power
> + Management system device.
> + This driver provides common support for accessing the device
> + through i2c interface. The device supports multiple sub-devices
> + like interrupts, RTC, LDO and DCDC regulators, onkey.
> +
> config MFD_SEC_CORE
> bool "SAMSUNG Electronics PMIC Series Support"
> depends on I2C=y
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index f001487..dbc28e7 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -160,6 +160,7 @@ obj-$(CONFIG_MFD_INTEL_MSIC)+= intel_msic.o
> obj-$(CONFIG_MFD_PALMAS) += palmas.o
> obj-$(CONFIG_MFD_VIPERBOARD)+= viperboard.o
> obj-$(CONFIG_MFD_RC5T583) += rc5t583.o rc5t583-irq.o
> +obj-$(CONFIG_MFD_RK808)+= rk808.o
> obj-$(CONFIG_MFD_SEC_CORE) += sec-core.o sec-irq.o
> obj-$(CONFIG_MFD_SYSCON) += syscon.o
> obj-$(CONFIG_MFD_LM3533) += lm3533-core.o lm3533-ctrlbank.o
> diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
> new file mode 100644
> index 000..667cfdf
> --- /dev/null
> +++ b/drivers/mfd/rk808.c
> @@ -0,0 +1,297 @@
> +/*
> + * Mfd core driver for Rockchip RK808
> + *
> + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * Author: Chris Zhong
> + * Author: Zhang Qing
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +struct rk808_reg_data {
> + int addr;
> + int mask;
> + int value;
> +};
> +
> +static struct rk808 *g_rk808;
> +
> +static struct resource rtc_resources[] = {
> + {
> + .start = RK808_IRQ_RTC_ALARM,
> + .end= RK808_IRQ_RTC_ALARM,
> + .flags = IORESOURCE_IRQ,
> + }
> +};
> +
> +static const struct mfd_cell rk808s[] = {
> + {
> + .name = "rk808-regulator",
> + },
> + {
> + .name = "rk808-rtc",
> + .num_resources = ARRAY_SIZE(rtc_resources),
> + .resources = _resources[0],
> + },
> + {
> + .name = "rk808-clkout",
> + },
> +};
> +
> +static const struct rk808_reg_data pre_init_reg[] = {
> + {RK808_BUCK3_CONFIG_REG, BUCK_ILMIN_MASK, BUCK_ILMIN_150MA},
> + {RK808_BUCK4_CONFIG_REG, BUCK_ILMIN_MASK, BUCK_ILMIN_200MA},
> + {RK808_BOOST_CONFIG_REG, BOOST_ILMIN_MASK, BOOST_ILMIN_100MA},
> + {RK808_BUCK1_CONFIG_REG, BUCK1_RATE_MASK, BUCK_ILMIN_200MA},
> + {RK808_BUCK2_CONFIG_REG, BUCK2_RATE_MASK, BUCK_ILMIN_200MA},
> + {RK808_VB_MON_REG, MASK_ALL, VB_LO_ACT | VB_LO_SEL_3500MV},
> +
Re: [PATCH v2 2/5] MFD: RK808: Add new mfd driver for RK808
On Wed, 20 Aug 2014, Chris Zhong wrote:
> The RK808 chip is a power management IC for multimedia and handheld
> devices. It contains the following components:
>
> - Regulators
> - RTC
>
> The rk808 core driver is registered as a platform driver and provides
> communication through I2C with the host device for the different
> components.
>
> Signed-off-by: Chris Zhong
>
> ---
>
> Changes in v2:
> Adviced by Mark Browm:
> - change of_find_node_by_name to find_child_by_name
> - use RK808_NUM_REGULATORS as the name of the constant
> - create a pdata when missing platform data
> - use the rk808_reg name to supply_regulator name
> - replace regulator_register with devm_regulator_register
> - some other problem with coding style
>
> drivers/mfd/Kconfig | 13 ++
> drivers/mfd/Makefile |1 +
> drivers/mfd/rk808.c | 297
> +
> include/linux/mfd/rk808.h | 219 +
> 4 files changed, 530 insertions(+)
> create mode 100644 drivers/mfd/rk808.c
> create mode 100644 include/linux/mfd/rk808.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index de5abf2..1df133e 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -582,6 +582,19 @@ config MFD_RC5T583
> Additional drivers must be enabled in order to use the
> different functionality of the device.
>
> +config MFD_RK808
> + tristate "Rockchip RK808 Power Management chip"
> + depends on I2C
> + select MFD_CORE
> + select REGMAP_I2C
> + select REGMAP_IRQ
> + help
<-- Use more of the allotted space
--->
> + Select this option to get support for the RK808 Power
> + Management system device.
What's a 'system device', and how does that differ to a controller?
> + This driver provides common support for accessing the device
> + through i2c interface. The device supports multiple sub-devices
s/i2c/I2C/
> + like interrupts, RTC, LDO and DCDC regulators, onkey.
s/like/including/
I would s/and/&/, then put an "and" before "onkey".
this, this, this 'and' that.
> config MFD_SEC_CORE
> bool "SAMSUNG Electronics PMIC Series Support"
> depends on I2C=y
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index f001487..dbc28e7 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -160,6 +160,7 @@ obj-$(CONFIG_MFD_INTEL_MSIC) += intel_msic.o
> obj-$(CONFIG_MFD_PALMAS) += palmas.o
> obj-$(CONFIG_MFD_VIPERBOARD)+= viperboard.o
> obj-$(CONFIG_MFD_RC5T583)+= rc5t583.o rc5t583-irq.o
> +obj-$(CONFIG_MFD_RK808) += rk808.o
> obj-$(CONFIG_MFD_SEC_CORE) += sec-core.o sec-irq.o
> obj-$(CONFIG_MFD_SYSCON) += syscon.o
> obj-$(CONFIG_MFD_LM3533) += lm3533-core.o lm3533-ctrlbank.o
> diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
> new file mode 100644
> index 000..667cfdf
> --- /dev/null
> +++ b/drivers/mfd/rk808.c
> @@ -0,0 +1,297 @@
> +/*
> + * Mfd core driver for Rockchip RK808
s/Mfd/MFD
> + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * Author: Chris Zhong
> + * Author: Zhang Qing
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.
> + *
Remove this line.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
I'm pretty sure you don't need all of these includes.
Remove the ones you're not using.
> +struct rk808_reg_data {
> + int addr;
> + int mask;
> + int value;
> +};
> +
> +static struct rk808 *g_rk808;
Grim.
> +static struct resource rtc_resources[] = {
> + {
> + .start = RK808_IRQ_RTC_ALARM,
> + .end= RK808_IRQ_RTC_ALARM,
> + .flags = IORESOURCE_IRQ,
> + }
> +};
> +
> +static const struct mfd_cell rk808s[] = {
> + {
> + .name = "rk808-regulator",
> + },
> + {
> + .name = "rk808-rtc",
> + .num_resources = ARRAY_SIZE(rtc_resources),
> + .resources = _resources[0],
> + },
> + {
> + .name = "rk808-clkout",
> + },
Can you reorder these, with the single liners at the start and
actually make them one line, so:
{ .name = "rk808-clkout" },
{ .name = "rk808-regulator" },
{
.name = "rk808-rtc",
.num_resources =
Re: [PATCH v2 2/5] MFD: RK808: Add new mfd driver for RK808
On Wed, 20 Aug 2014, Chris Zhong wrote:
The RK808 chip is a power management IC for multimedia and handheld
devices. It contains the following components:
- Regulators
- RTC
The rk808 core driver is registered as a platform driver and provides
communication through I2C with the host device for the different
components.
Signed-off-by: Chris Zhong z...@rock-chips.com
---
Changes in v2:
Adviced by Mark Browm:
- change of_find_node_by_name to find_child_by_name
- use RK808_NUM_REGULATORS as the name of the constant
- create a pdata when missing platform data
- use the rk808_reg name to supply_regulator name
- replace regulator_register with devm_regulator_register
- some other problem with coding style
drivers/mfd/Kconfig | 13 ++
drivers/mfd/Makefile |1 +
drivers/mfd/rk808.c | 297
+
include/linux/mfd/rk808.h | 219 +
4 files changed, 530 insertions(+)
create mode 100644 drivers/mfd/rk808.c
create mode 100644 include/linux/mfd/rk808.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de5abf2..1df133e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -582,6 +582,19 @@ config MFD_RC5T583
Additional drivers must be enabled in order to use the
different functionality of the device.
+config MFD_RK808
+ tristate Rockchip RK808 Power Management chip
+ depends on I2C
+ select MFD_CORE
+ select REGMAP_I2C
+ select REGMAP_IRQ
+ help
-- Use more of the allotted space
---
+ Select this option to get support for the RK808 Power
+ Management system device.
What's a 'system device', and how does that differ to a controller?
+ This driver provides common support for accessing the device
+ through i2c interface. The device supports multiple sub-devices
s/i2c/I2C/
+ like interrupts, RTC, LDO and DCDC regulators, onkey.
s/like/including/
I would s/and//, then put an and before onkey.
this, this, this 'and' that.
config MFD_SEC_CORE
bool SAMSUNG Electronics PMIC Series Support
depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f001487..dbc28e7 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -160,6 +160,7 @@ obj-$(CONFIG_MFD_INTEL_MSIC) += intel_msic.o
obj-$(CONFIG_MFD_PALMAS) += palmas.o
obj-$(CONFIG_MFD_VIPERBOARD)+= viperboard.o
obj-$(CONFIG_MFD_RC5T583)+= rc5t583.o rc5t583-irq.o
+obj-$(CONFIG_MFD_RK808) += rk808.o
obj-$(CONFIG_MFD_SEC_CORE) += sec-core.o sec-irq.o
obj-$(CONFIG_MFD_SYSCON) += syscon.o
obj-$(CONFIG_MFD_LM3533) += lm3533-core.o lm3533-ctrlbank.o
diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
new file mode 100644
index 000..667cfdf
--- /dev/null
+++ b/drivers/mfd/rk808.c
@@ -0,0 +1,297 @@
+/*
+ * Mfd core driver for Rockchip RK808
s/Mfd/MFD
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * Author: Chris Zhong z...@rock-chips.com
+ * Author: Zhang Qing zhangq...@rock-chips.com
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
Remove this line.
+ */
+
+#include linux/bug.h
+#include linux/delay.h
+#include linux/err.h
+#include linux/i2c.h
+#include linux/kernel.h
+#include linux/interrupt.h
+#include linux/mfd/rk808.h
+#include linux/mfd/core.h
+#include linux/mutex.h
+#include linux/module.h
+#include linux/of_irq.h
+#include linux/of_gpio.h
+#include linux/of.h
+#include linux/of_device.h
+#include linux/regulator/driver.h
+#include linux/regulator/of_regulator.h
+#include linux/regulator/driver.h
+#include linux/regulator/machine.h
+#include linux/regmap.h
+#include linux/slab.h
I'm pretty sure you don't need all of these includes.
Remove the ones you're not using.
+struct rk808_reg_data {
+ int addr;
+ int mask;
+ int value;
+};
+
+static struct rk808 *g_rk808;
Grim.
+static struct resource rtc_resources[] = {
+ {
+ .start = RK808_IRQ_RTC_ALARM,
+ .end= RK808_IRQ_RTC_ALARM,
+ .flags = IORESOURCE_IRQ,
+ }
+};
+
+static const struct mfd_cell rk808s[] = {
+ {
+ .name = rk808-regulator,
+ },
+ {
+ .name = rk808-rtc,
+ .num_resources = ARRAY_SIZE(rtc_resources),
+ .resources = rtc_resources[0],
+ },
+ {
+ .name = rk808-clkout,
+
Re: [PATCH v2 2/5] MFD: RK808: Add new mfd driver for RK808
Chris,
On Tue, Aug 19, 2014 at 8:31 PM, Chris Zhong z...@rock-chips.com wrote:
The RK808 chip is a power management IC for multimedia and handheld
devices. It contains the following components:
- Regulators
- RTC
The rk808 core driver is registered as a platform driver and provides
communication through I2C with the host device for the different
components.
Signed-off-by: Chris Zhong z...@rock-chips.com
---
Changes in v2:
Adviced by Mark Browm:
- change of_find_node_by_name to find_child_by_name
- use RK808_NUM_REGULATORS as the name of the constant
- create a pdata when missing platform data
- use the rk808_reg name to supply_regulator name
- replace regulator_register with devm_regulator_register
- some other problem with coding style
drivers/mfd/Kconfig | 13 ++
drivers/mfd/Makefile |1 +
drivers/mfd/rk808.c | 297
+
include/linux/mfd/rk808.h | 219 +
4 files changed, 530 insertions(+)
create mode 100644 drivers/mfd/rk808.c
create mode 100644 include/linux/mfd/rk808.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de5abf2..1df133e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -582,6 +582,19 @@ config MFD_RC5T583
Additional drivers must be enabled in order to use the
different functionality of the device.
+config MFD_RK808
+ tristate Rockchip RK808 Power Management chip
+ depends on I2C
+ select MFD_CORE
+ select REGMAP_I2C
+ select REGMAP_IRQ
+ help
+ Select this option to get support for the RK808 Power
+ Management system device.
+ This driver provides common support for accessing the device
+ through i2c interface. The device supports multiple sub-devices
+ like interrupts, RTC, LDO and DCDC regulators, onkey.
+
config MFD_SEC_CORE
bool SAMSUNG Electronics PMIC Series Support
depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f001487..dbc28e7 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -160,6 +160,7 @@ obj-$(CONFIG_MFD_INTEL_MSIC)+= intel_msic.o
obj-$(CONFIG_MFD_PALMAS) += palmas.o
obj-$(CONFIG_MFD_VIPERBOARD)+= viperboard.o
obj-$(CONFIG_MFD_RC5T583) += rc5t583.o rc5t583-irq.o
+obj-$(CONFIG_MFD_RK808)+= rk808.o
obj-$(CONFIG_MFD_SEC_CORE) += sec-core.o sec-irq.o
obj-$(CONFIG_MFD_SYSCON) += syscon.o
obj-$(CONFIG_MFD_LM3533) += lm3533-core.o lm3533-ctrlbank.o
diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
new file mode 100644
index 000..667cfdf
--- /dev/null
+++ b/drivers/mfd/rk808.c
@@ -0,0 +1,297 @@
+/*
+ * Mfd core driver for Rockchip RK808
+ *
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * Author: Chris Zhong z...@rock-chips.com
+ * Author: Zhang Qing zhangq...@rock-chips.com
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ */
+
+#include linux/bug.h
+#include linux/delay.h
+#include linux/err.h
+#include linux/i2c.h
+#include linux/kernel.h
+#include linux/interrupt.h
+#include linux/mfd/rk808.h
+#include linux/mfd/core.h
+#include linux/mutex.h
+#include linux/module.h
+#include linux/of_irq.h
+#include linux/of_gpio.h
+#include linux/of.h
+#include linux/of_device.h
+#include linux/regulator/driver.h
+#include linux/regulator/of_regulator.h
+#include linux/regulator/driver.h
+#include linux/regulator/machine.h
+#include linux/regmap.h
+#include linux/slab.h
+
+struct rk808_reg_data {
+ int addr;
+ int mask;
+ int value;
+};
+
+static struct rk808 *g_rk808;
+
+static struct resource rtc_resources[] = {
+ {
+ .start = RK808_IRQ_RTC_ALARM,
+ .end= RK808_IRQ_RTC_ALARM,
+ .flags = IORESOURCE_IRQ,
+ }
+};
+
+static const struct mfd_cell rk808s[] = {
+ {
+ .name = rk808-regulator,
+ },
+ {
+ .name = rk808-rtc,
+ .num_resources = ARRAY_SIZE(rtc_resources),
+ .resources = rtc_resources[0],
+ },
+ {
+ .name = rk808-clkout,
+ },
+};
+
+static const struct rk808_reg_data pre_init_reg[] = {
+ {RK808_BUCK3_CONFIG_REG, BUCK_ILMIN_MASK, BUCK_ILMIN_150MA},
+ {RK808_BUCK4_CONFIG_REG, BUCK_ILMIN_MASK, BUCK_ILMIN_200MA},
+ {RK808_BOOST_CONFIG_REG, BOOST_ILMIN_MASK,
[PATCH v2 2/5] MFD: RK808: Add new mfd driver for RK808
The RK808 chip is a power management IC for multimedia and handheld
devices. It contains the following components:
- Regulators
- RTC
The rk808 core driver is registered as a platform driver and provides
communication through I2C with the host device for the different
components.
Signed-off-by: Chris Zhong
---
Changes in v2:
Adviced by Mark Browm:
- change of_find_node_by_name to find_child_by_name
- use RK808_NUM_REGULATORS as the name of the constant
- create a pdata when missing platform data
- use the rk808_reg name to supply_regulator name
- replace regulator_register with devm_regulator_register
- some other problem with coding style
drivers/mfd/Kconfig | 13 ++
drivers/mfd/Makefile |1 +
drivers/mfd/rk808.c | 297 +
include/linux/mfd/rk808.h | 219 +
4 files changed, 530 insertions(+)
create mode 100644 drivers/mfd/rk808.c
create mode 100644 include/linux/mfd/rk808.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de5abf2..1df133e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -582,6 +582,19 @@ config MFD_RC5T583
Additional drivers must be enabled in order to use the
different functionality of the device.
+config MFD_RK808
+ tristate "Rockchip RK808 Power Management chip"
+ depends on I2C
+ select MFD_CORE
+ select REGMAP_I2C
+ select REGMAP_IRQ
+ help
+ Select this option to get support for the RK808 Power
+ Management system device.
+ This driver provides common support for accessing the device
+ through i2c interface. The device supports multiple sub-devices
+ like interrupts, RTC, LDO and DCDC regulators, onkey.
+
config MFD_SEC_CORE
bool "SAMSUNG Electronics PMIC Series Support"
depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f001487..dbc28e7 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -160,6 +160,7 @@ obj-$(CONFIG_MFD_INTEL_MSIC)+= intel_msic.o
obj-$(CONFIG_MFD_PALMAS) += palmas.o
obj-$(CONFIG_MFD_VIPERBOARD)+= viperboard.o
obj-$(CONFIG_MFD_RC5T583) += rc5t583.o rc5t583-irq.o
+obj-$(CONFIG_MFD_RK808)+= rk808.o
obj-$(CONFIG_MFD_SEC_CORE) += sec-core.o sec-irq.o
obj-$(CONFIG_MFD_SYSCON) += syscon.o
obj-$(CONFIG_MFD_LM3533) += lm3533-core.o lm3533-ctrlbank.o
diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
new file mode 100644
index 000..667cfdf
--- /dev/null
+++ b/drivers/mfd/rk808.c
@@ -0,0 +1,297 @@
+/*
+ * Mfd core driver for Rockchip RK808
+ *
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * Author: Chris Zhong
+ * Author: Zhang Qing
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct rk808_reg_data {
+ int addr;
+ int mask;
+ int value;
+};
+
+static struct rk808 *g_rk808;
+
+static struct resource rtc_resources[] = {
+ {
+ .start = RK808_IRQ_RTC_ALARM,
+ .end= RK808_IRQ_RTC_ALARM,
+ .flags = IORESOURCE_IRQ,
+ }
+};
+
+static const struct mfd_cell rk808s[] = {
+ {
+ .name = "rk808-regulator",
+ },
+ {
+ .name = "rk808-rtc",
+ .num_resources = ARRAY_SIZE(rtc_resources),
+ .resources = _resources[0],
+ },
+ {
+ .name = "rk808-clkout",
+ },
+};
+
+static const struct rk808_reg_data pre_init_reg[] = {
+ {RK808_BUCK3_CONFIG_REG, BUCK_ILMIN_MASK, BUCK_ILMIN_150MA},
+ {RK808_BUCK4_CONFIG_REG, BUCK_ILMIN_MASK, BUCK_ILMIN_200MA},
+ {RK808_BOOST_CONFIG_REG, BOOST_ILMIN_MASK, BOOST_ILMIN_100MA},
+ {RK808_BUCK1_CONFIG_REG, BUCK1_RATE_MASK, BUCK_ILMIN_200MA},
+ {RK808_BUCK2_CONFIG_REG, BUCK2_RATE_MASK, BUCK_ILMIN_200MA},
+ {RK808_VB_MON_REG, MASK_ALL, VB_LO_ACT | VB_LO_SEL_3500MV},
+ {RK808_INT_STS_REG1, MASK_NONE, 0},
+ {RK808_INT_STS_REG2, MASK_NONE, 0},
+};
+
+static const struct regmap_irq rk808_irqs[] = {
+ /* INT_STS */
+ [RK808_IRQ_VOUT_LO] = {
+ .mask = RK808_IRQ_VOUT_LO_MSK,
+ .reg_offset = 0,
+ },
+ [RK808_IRQ_VB_LO] = {
+ .mask = RK808_IRQ_VB_LO_MSK,
+
[PATCH v2 2/5] MFD: RK808: Add new mfd driver for RK808
The RK808 chip is a power management IC for multimedia and handheld
devices. It contains the following components:
- Regulators
- RTC
The rk808 core driver is registered as a platform driver and provides
communication through I2C with the host device for the different
components.
Signed-off-by: Chris Zhong z...@rock-chips.com
---
Changes in v2:
Adviced by Mark Browm:
- change of_find_node_by_name to find_child_by_name
- use RK808_NUM_REGULATORS as the name of the constant
- create a pdata when missing platform data
- use the rk808_reg name to supply_regulator name
- replace regulator_register with devm_regulator_register
- some other problem with coding style
drivers/mfd/Kconfig | 13 ++
drivers/mfd/Makefile |1 +
drivers/mfd/rk808.c | 297 +
include/linux/mfd/rk808.h | 219 +
4 files changed, 530 insertions(+)
create mode 100644 drivers/mfd/rk808.c
create mode 100644 include/linux/mfd/rk808.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de5abf2..1df133e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -582,6 +582,19 @@ config MFD_RC5T583
Additional drivers must be enabled in order to use the
different functionality of the device.
+config MFD_RK808
+ tristate Rockchip RK808 Power Management chip
+ depends on I2C
+ select MFD_CORE
+ select REGMAP_I2C
+ select REGMAP_IRQ
+ help
+ Select this option to get support for the RK808 Power
+ Management system device.
+ This driver provides common support for accessing the device
+ through i2c interface. The device supports multiple sub-devices
+ like interrupts, RTC, LDO and DCDC regulators, onkey.
+
config MFD_SEC_CORE
bool SAMSUNG Electronics PMIC Series Support
depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f001487..dbc28e7 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -160,6 +160,7 @@ obj-$(CONFIG_MFD_INTEL_MSIC)+= intel_msic.o
obj-$(CONFIG_MFD_PALMAS) += palmas.o
obj-$(CONFIG_MFD_VIPERBOARD)+= viperboard.o
obj-$(CONFIG_MFD_RC5T583) += rc5t583.o rc5t583-irq.o
+obj-$(CONFIG_MFD_RK808)+= rk808.o
obj-$(CONFIG_MFD_SEC_CORE) += sec-core.o sec-irq.o
obj-$(CONFIG_MFD_SYSCON) += syscon.o
obj-$(CONFIG_MFD_LM3533) += lm3533-core.o lm3533-ctrlbank.o
diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
new file mode 100644
index 000..667cfdf
--- /dev/null
+++ b/drivers/mfd/rk808.c
@@ -0,0 +1,297 @@
+/*
+ * Mfd core driver for Rockchip RK808
+ *
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * Author: Chris Zhong z...@rock-chips.com
+ * Author: Zhang Qing zhangq...@rock-chips.com
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ */
+
+#include linux/bug.h
+#include linux/delay.h
+#include linux/err.h
+#include linux/i2c.h
+#include linux/kernel.h
+#include linux/interrupt.h
+#include linux/mfd/rk808.h
+#include linux/mfd/core.h
+#include linux/mutex.h
+#include linux/module.h
+#include linux/of_irq.h
+#include linux/of_gpio.h
+#include linux/of.h
+#include linux/of_device.h
+#include linux/regulator/driver.h
+#include linux/regulator/of_regulator.h
+#include linux/regulator/driver.h
+#include linux/regulator/machine.h
+#include linux/regmap.h
+#include linux/slab.h
+
+struct rk808_reg_data {
+ int addr;
+ int mask;
+ int value;
+};
+
+static struct rk808 *g_rk808;
+
+static struct resource rtc_resources[] = {
+ {
+ .start = RK808_IRQ_RTC_ALARM,
+ .end= RK808_IRQ_RTC_ALARM,
+ .flags = IORESOURCE_IRQ,
+ }
+};
+
+static const struct mfd_cell rk808s[] = {
+ {
+ .name = rk808-regulator,
+ },
+ {
+ .name = rk808-rtc,
+ .num_resources = ARRAY_SIZE(rtc_resources),
+ .resources = rtc_resources[0],
+ },
+ {
+ .name = rk808-clkout,
+ },
+};
+
+static const struct rk808_reg_data pre_init_reg[] = {
+ {RK808_BUCK3_CONFIG_REG, BUCK_ILMIN_MASK, BUCK_ILMIN_150MA},
+ {RK808_BUCK4_CONFIG_REG, BUCK_ILMIN_MASK, BUCK_ILMIN_200MA},
+ {RK808_BOOST_CONFIG_REG, BOOST_ILMIN_MASK, BOOST_ILMIN_100MA},
+ {RK808_BUCK1_CONFIG_REG, BUCK1_RATE_MASK, BUCK_ILMIN_200MA},
+ {RK808_BUCK2_CONFIG_REG, BUCK2_RATE_MASK, BUCK_ILMIN_200MA},
+ {RK808_VB_MON_REG, MASK_ALL, VB_LO_ACT |
