Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Tue, Nov 10, 2015 at 01:40:38PM -0600, Andrew F. Davis wrote: > On 11/10/2015 12:44 PM, Mark Brown wrote: > >There's also the third option where we don't have any compatible strings > >in the subnodes at all. > Ok, two, but would you really want to go that way? Matching by node name costs >

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Tue, Nov 10, 2015 at 01:40:38PM -0600, Andrew F. Davis wrote: > On 11/10/2015 12:44 PM, Mark Brown wrote: > >There's also the third option where we don't have any compatible strings > >in the subnodes at all. > Ok, two, but would you really want to go that way? Matching by node name costs >

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 11/10/2015 12:44 PM, Mark Brown wrote: On Tue, Nov 10, 2015 at 11:52:12AM -0600, Andrew F. Davis wrote: Anyway, All I'm trying to do here is keep things clean in the DT. We only have one consistent option: No, not really. Match all sub parts by compatible: Or we end up with some

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Tue, Nov 10, 2015 at 11:52:12AM -0600, Andrew F. Davis wrote: > Anyway, All I'm trying to do here is keep things clean in the DT. We only have > one consistent option: No, not really. > Match all sub parts by compatible: > Or we end up with some hybrid approach, matching some on node name,

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 11/10/2015 11:04 AM, Mark Brown wrote: On Tue, Nov 10, 2015 at 10:47:33AM -0600, Andrew F. Davis wrote: On 11/10/2015 03:57 AM, Mark Brown wrote: Of course this is a negative review of the binding! What on earth did you think my feedback meant? The driver and the binding go together.

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Tue, Nov 10, 2015 at 10:47:33AM -0600, Andrew F. Davis wrote: > On 11/10/2015 03:57 AM, Mark Brown wrote: > >Of course this is a negative review of the binding! What on earth did > >you think my feedback meant? The driver and the binding go together. > The bindings should be

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 11/10/2015 03:57 AM, Mark Brown wrote: On Mon, Nov 09, 2015 at 11:41:20AM -0600, Andrew F. Davis wrote: On 11/06/2015 03:16 PM, Mark Brown wrote: There are cases where it's useful where we're abstracting something and gaining some meaningful reuse. This really does not appear to be one

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Mon, Nov 09, 2015 at 11:41:20AM -0600, Andrew F. Davis wrote: > On 11/06/2015 03:16 PM, Mark Brown wrote: > >There are cases where it's useful where we're abstracting something and > >gaining some meaningful reuse. This really does not appear to be one of > >those cases, there are no

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 11/10/2015 03:57 AM, Mark Brown wrote: On Mon, Nov 09, 2015 at 11:41:20AM -0600, Andrew F. Davis wrote: On 11/06/2015 03:16 PM, Mark Brown wrote: There are cases where it's useful where we're abstracting something and gaining some meaningful reuse. This really does not appear to be one

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Tue, Nov 10, 2015 at 10:47:33AM -0600, Andrew F. Davis wrote: > On 11/10/2015 03:57 AM, Mark Brown wrote: > >Of course this is a negative review of the binding! What on earth did > >you think my feedback meant? The driver and the binding go together. > The bindings should be

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 11/10/2015 11:04 AM, Mark Brown wrote: On Tue, Nov 10, 2015 at 10:47:33AM -0600, Andrew F. Davis wrote: On 11/10/2015 03:57 AM, Mark Brown wrote: Of course this is a negative review of the binding! What on earth did you think my feedback meant? The driver and the binding go together.

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Tue, Nov 10, 2015 at 11:52:12AM -0600, Andrew F. Davis wrote: > Anyway, All I'm trying to do here is keep things clean in the DT. We only have > one consistent option: No, not really. > Match all sub parts by compatible: > Or we end up with some hybrid approach, matching some on node name,

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 11/10/2015 12:44 PM, Mark Brown wrote: On Tue, Nov 10, 2015 at 11:52:12AM -0600, Andrew F. Davis wrote: Anyway, All I'm trying to do here is keep things clean in the DT. We only have one consistent option: No, not really. Match all sub parts by compatible: Or we end up with some

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Mon, Nov 09, 2015 at 11:41:20AM -0600, Andrew F. Davis wrote: > On 11/06/2015 03:16 PM, Mark Brown wrote: > >There are cases where it's useful where we're abstracting something and > >gaining some meaningful reuse. This really does not appear to be one of > >those cases, there are no

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 11/06/2015 03:16 PM, Mark Brown wrote: On Fri, Nov 06, 2015 at 12:10:45PM -0600, Andrew F. Davis wrote: On 11/06/2015 04:43 AM, Mark Brown wrote: No, you need to fix the bug that is causing dev->of_node to be populated for the MFD function device. Probably the issue is that you have put

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 11/06/2015 03:16 PM, Mark Brown wrote: On Fri, Nov 06, 2015 at 12:10:45PM -0600, Andrew F. Davis wrote: On 11/06/2015 04:43 AM, Mark Brown wrote: No, you need to fix the bug that is causing dev->of_node to be populated for the MFD function device. Probably the issue is that you have put

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Fri, Nov 06, 2015 at 12:10:45PM -0600, Andrew F. Davis wrote: > On 11/06/2015 04:43 AM, Mark Brown wrote: > >No, you need to fix the bug that is causing dev->of_node to be populated > >for the MFD function device. Probably the issue is that you have put > >this pointless compatible string in

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 11/06/2015 04:43 AM, Mark Brown wrote: On Thu, Nov 05, 2015 at 12:04:00PM -0600, Andrew F. Davis wrote: On 11/05/2015 04:14 AM, Mark Brown wrote: That sounds like a bug to me, it'll have broken a bunch of existing devices. Most OF drivers have the OF MODALIAS. That's nice but not

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Thu, Nov 05, 2015 at 12:04:00PM -0600, Andrew F. Davis wrote: > On 11/05/2015 04:14 AM, Mark Brown wrote: > >That sounds like a bug to me, it'll have broken a bunch of existing > >devices. > Most OF drivers have the OF MODALIAS. That's nice but not relevant to non-OF devices. >

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Thu, Nov 05, 2015 at 12:04:00PM -0600, Andrew F. Davis wrote: > On 11/05/2015 04:14 AM, Mark Brown wrote: > >That sounds like a bug to me, it'll have broken a bunch of existing > >devices. > Most OF drivers have the OF MODALIAS. That's nice but not relevant to non-OF devices. >

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 11/06/2015 04:43 AM, Mark Brown wrote: On Thu, Nov 05, 2015 at 12:04:00PM -0600, Andrew F. Davis wrote: On 11/05/2015 04:14 AM, Mark Brown wrote: That sounds like a bug to me, it'll have broken a bunch of existing devices. Most OF drivers have the OF MODALIAS. That's nice but not

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Fri, Nov 06, 2015 at 12:10:45PM -0600, Andrew F. Davis wrote: > On 11/06/2015 04:43 AM, Mark Brown wrote: > >No, you need to fix the bug that is causing dev->of_node to be populated > >for the MFD function device. Probably the issue is that you have put > >this pointless compatible string in

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 11/05/2015 04:14 AM, Mark Brown wrote: On Wed, Nov 04, 2015 at 09:35:26AM -0600, Andrew F. Davis wrote: Something I just noticed, when I remove this table, module loading stops working, even with 'MODULE_ALIAS("platform:tps65912-regulator");'. It looks like when DT is enabled

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Wed, Nov 04, 2015 at 09:35:26AM -0600, Andrew F. Davis wrote: > Something I just noticed, when I remove this table, module loading stops > working, even with 'MODULE_ALIAS("platform:tps65912-regulator");'. It > looks like when DT is enabled platform_uevent (drivers/base/platform.c:787) > only

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Wed, Nov 04, 2015 at 09:35:26AM -0600, Andrew F. Davis wrote: > Something I just noticed, when I remove this table, module loading stops > working, even with 'MODULE_ALIAS("platform:tps65912-regulator");'. It > looks like when DT is enabled platform_uevent (drivers/base/platform.c:787) > only

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 11/05/2015 04:14 AM, Mark Brown wrote: On Wed, Nov 04, 2015 at 09:35:26AM -0600, Andrew F. Davis wrote: Something I just noticed, when I remove this table, module loading stops working, even with 'MODULE_ALIAS("platform:tps65912-regulator");'. It looks like when DT is enabled

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 10/22/2015 11:47 AM, Mark Brown wrote: On Thu, Oct 01, 2015 at 03:37:53PM -0500, Andrew F. Davis wrote: +static const struct of_device_id tps65912_regulator_of_match_table[] = { + { .compatible = "ti,tps65912-regulator", }, + { /* sentinel */ }, +}; +MODULE_DEVICE_TABLE(of,

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 10/22/2015 11:47 AM, Mark Brown wrote: On Thu, Oct 01, 2015 at 03:37:53PM -0500, Andrew F. Davis wrote: +static const struct of_device_id tps65912_regulator_of_match_table[] = { + { .compatible = "ti,tps65912-regulator", }, + { /* sentinel */ }, +}; +MODULE_DEVICE_TABLE(of,

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 10/26/2015 07:16 PM, Mark Brown wrote: On Mon, Oct 26, 2015 at 10:47:41AM -0500, Andrew F. Davis wrote: On 10/25/2015 07:43 PM, Mark Brown wrote: .of_compatible = "mediatek,mt6397-regulator", This is in the MFD, this is not used in actual systems. Not sure what you mean by "actual

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 10/26/2015 07:16 PM, Mark Brown wrote: On Mon, Oct 26, 2015 at 10:47:41AM -0500, Andrew F. Davis wrote: On 10/25/2015 07:43 PM, Mark Brown wrote: .of_compatible = "mediatek,mt6397-regulator", This is in the MFD, this is not used in actual systems. Not sure what you mean by "actual

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Mon, Oct 26, 2015 at 10:47:41AM -0500, Andrew F. Davis wrote: > On 10/25/2015 07:43 PM, Mark Brown wrote: > >>.of_compatible = "mediatek,mt6397-regulator", > >This is in the MFD, this is not used in actual systems. > Not sure what you mean by "actual systems", it looks like these > use it?:

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 10/25/2015 07:43 PM, Mark Brown wrote: On Sun, Oct 25, 2015 at 03:45:43PM -0500, Andrew F. Davis wrote: On 10/24/2015 05:14 PM, Mark Brown wrote: Tbe binding document is buggy and doesn't reflect the code, there's no compatible string in the driver. Sure there is:

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Mon, Oct 26, 2015 at 10:47:41AM -0500, Andrew F. Davis wrote: > On 10/25/2015 07:43 PM, Mark Brown wrote: > >>.of_compatible = "mediatek,mt6397-regulator", > >This is in the MFD, this is not used in actual systems. > Not sure what you mean by "actual systems", it looks like these > use it?:

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 10/25/2015 07:43 PM, Mark Brown wrote: On Sun, Oct 25, 2015 at 03:45:43PM -0500, Andrew F. Davis wrote: On 10/24/2015 05:14 PM, Mark Brown wrote: Tbe binding document is buggy and doesn't reflect the code, there's no compatible string in the driver. Sure there is:

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Sun, Oct 25, 2015 at 03:45:43PM -0500, Andrew F. Davis wrote: > On 10/24/2015 05:14 PM, Mark Brown wrote: > >Tbe binding document is buggy and doesn't reflect the code, there's no > >compatible string in the driver. > Sure there is: > drivers/mfd/mt6397-core.c:48: > .of_compatible =

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 10/24/2015 05:14 PM, Mark Brown wrote: On Fri, Oct 23, 2015 at 07:11:56PM -0500, Andrew F. Davis wrote: On 10/23/2015 06:18 PM, Mark Brown wrote: mt6397 doesn't do this, it doesn't have a compatible string at all (it's doing what I'm recommending that you do). The SPMI devices are

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Sun, Oct 25, 2015 at 03:45:43PM -0500, Andrew F. Davis wrote: > On 10/24/2015 05:14 PM, Mark Brown wrote: > >Tbe binding document is buggy and doesn't reflect the code, there's no > >compatible string in the driver. > Sure there is: > drivers/mfd/mt6397-core.c:48: > .of_compatible =

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 10/24/2015 05:14 PM, Mark Brown wrote: On Fri, Oct 23, 2015 at 07:11:56PM -0500, Andrew F. Davis wrote: On 10/23/2015 06:18 PM, Mark Brown wrote: mt6397 doesn't do this, it doesn't have a compatible string at all (it's doing what I'm recommending that you do). The SPMI devices are

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Fri, Oct 23, 2015 at 07:11:56PM -0500, Andrew F. Davis wrote: > On 10/23/2015 06:18 PM, Mark Brown wrote: > >mt6397 doesn't do this, it doesn't have a compatible string at all (it's > >doing what I'm recommending that you do). The SPMI devices are > >standalone devices, their parent device is

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Fri, Oct 23, 2015 at 07:11:56PM -0500, Andrew F. Davis wrote: > On 10/23/2015 06:18 PM, Mark Brown wrote: > >mt6397 doesn't do this, it doesn't have a compatible string at all (it's > >doing what I'm recommending that you do). The SPMI devices are > >standalone devices, their parent device is

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 10/23/2015 06:18 PM, Mark Brown wrote: On Fri, Oct 23, 2015 at 07:46:39AM -0500, Andrew F. Davis wrote: I know just because other drivers do it doesn't mean it's a good idea, but this is not new for MFDs and it is done in other regulators as well (mt6397, tps659038, qcom,spmi, etc..).

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Fri, Oct 23, 2015 at 07:46:39AM -0500, Andrew F. Davis wrote: > I know just because other drivers do it doesn't mean it's a good idea, > but this is not new for MFDs and it is done in other regulators as well > (mt6397, tps659038, qcom,spmi, etc..). mt6397 doesn't do this, it doesn't have a

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 10/22/2015 11:47 AM, Mark Brown wrote: On Thu, Oct 01, 2015 at 03:37:53PM -0500, Andrew F. Davis wrote: +static const struct of_device_id tps65912_regulator_of_match_table[] = { + { .compatible = "ti,tps65912-regulator", }, + { /* sentinel */ }, +}; +MODULE_DEVICE_TABLE(of,

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 10/22/2015 11:47 AM, Mark Brown wrote: On Thu, Oct 01, 2015 at 03:37:53PM -0500, Andrew F. Davis wrote: +static const struct of_device_id tps65912_regulator_of_match_table[] = { + { .compatible = "ti,tps65912-regulator", }, + { /* sentinel */ }, +}; +MODULE_DEVICE_TABLE(of,

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 10/23/2015 06:18 PM, Mark Brown wrote: On Fri, Oct 23, 2015 at 07:46:39AM -0500, Andrew F. Davis wrote: I know just because other drivers do it doesn't mean it's a good idea, but this is not new for MFDs and it is done in other regulators as well (mt6397, tps659038, qcom,spmi, etc..).

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Fri, Oct 23, 2015 at 07:46:39AM -0500, Andrew F. Davis wrote: > I know just because other drivers do it doesn't mean it's a good idea, > but this is not new for MFDs and it is done in other regulators as well > (mt6397, tps659038, qcom,spmi, etc..). mt6397 doesn't do this, it doesn't have a

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Thu, Oct 01, 2015 at 03:37:53PM -0500, Andrew F. Davis wrote: > +static const struct of_device_id tps65912_regulator_of_match_table[] = { > + { .compatible = "ti,tps65912-regulator", }, > + { /* sentinel */ }, > +}; > +MODULE_DEVICE_TABLE(of, tps65912_regulator_of_match_table); Does

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On Thu, Oct 01, 2015 at 03:37:53PM -0500, Andrew F. Davis wrote: > +static const struct of_device_id tps65912_regulator_of_match_table[] = { > + { .compatible = "ti,tps65912-regulator", }, > + { /* sentinel */ }, > +}; > +MODULE_DEVICE_TABLE(of, tps65912_regulator_of_match_table); Does

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 10/01/2015 03:37 PM, Andrew F. Davis wrote: This patch adds support for TPS65912 PMIC regulators. The regulators set consists of 4 DCDCs and 10 LDOs. The output voltages are configurable and are meant to supply power to the main processor and other components. Signed-off-by: Andrew F. Davis

Re: [PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

On 10/01/2015 03:37 PM, Andrew F. Davis wrote: This patch adds support for TPS65912 PMIC regulators. The regulators set consists of 4 DCDCs and 10 LDOs. The output voltages are configurable and are meant to supply power to the main processor and other components. Signed-off-by: Andrew F. Davis

[PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

This patch adds support for TPS65912 PMIC regulators. The regulators set consists of 4 DCDCs and 10 LDOs. The output voltages are configurable and are meant to supply power to the main processor and other components. Signed-off-by: Andrew F. Davis --- drivers/regulator/Kconfig |

[PATCH v4 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

This patch adds support for TPS65912 PMIC regulators. The regulators set consists of 4 DCDCs and 10 LDOs. The output voltages are configurable and are meant to supply power to the main processor and other components. Signed-off-by: Andrew F. Davis --- drivers/regulator/Kconfig