| Message ID | 1433885881-19809-3-git-send-email-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede <hdegoede@redhat.com> wrote: > Add a cell for the usb power_supply part of the axp20x PMICs. > > Note that this cell is only for the usb power_supply part and not the > ac-power / battery-charger / rtc-backup-bat-charger bits. > > Depending on the board each of those must be enabled / disabled separately > in devicetree as most boards do not use all 4. So in dt each one needs its > own child-node of the axp20x node. Another reason for using separate child > nodes for each is so that other devicetree nodes can have a power-supply > property with a phandle referencing a node representing a single > power-supply. > > The decision to use a separate devicetree node for each is reflected on > the kernel side by each getting its own mfd-cell / platform_device and > platform-driver. > > Cc: Bruno Prémont <bonbons@linux-vserver.org> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/mfd/axp20x.c | 32 +++++++++++++++++++++++++++++++- > 1 file changed, 31 insertions(+), 1 deletion(-) > > diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c > index 6ffbc11..47ce233 100644 > --- a/drivers/mfd/axp20x.c > +++ b/drivers/mfd/axp20x.c > @@ -113,6 +113,30 @@ static struct resource axp20x_pek_resources[] = { > }, > }; > > +static struct resource axp20x_usb_power_supply_resources[] = { > + { > + .name = "VBUS_PLUGIN", > + .start = AXP20X_IRQ_VBUS_PLUGIN, > + .end = AXP20X_IRQ_VBUS_PLUGIN, > + .flags = IORESOURCE_IRQ, > + }, { > + .name = "VBUS_REMOVAL", > + .start = AXP20X_IRQ_VBUS_REMOVAL, > + .end = AXP20X_IRQ_VBUS_REMOVAL, > + .flags = IORESOURCE_IRQ, > + }, { > + .name = "VBUS_VALID", > + .start = AXP20X_IRQ_VBUS_VALID, > + .end = AXP20X_IRQ_VBUS_VALID, > + .flags = IORESOURCE_IRQ, > + }, { > + .name = "VBUS_NOT_VALID", > + .start = AXP20X_IRQ_VBUS_NOT_VALID, > + .end = AXP20X_IRQ_VBUS_NOT_VALID, > + .flags = IORESOURCE_IRQ, > + }, > +}; > + > static struct resource axp22x_pek_resources[] = { > { > .name = "PEK_DBR", > @@ -165,7 +189,7 @@ static const struct regmap_config axp20x_regmap_config = { > .val_bits = 8, > .wr_table = &axp20x_writeable_table, > .volatile_table = &axp20x_volatile_table, > - .max_register = AXP20X_FG_RES, > + .max_register = AXP20X_OCV(15), > .cache_type = REGCACHE_RBTREE, > }; > > @@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = { > .resources = axp20x_pek_resources, > }, { > .name = "axp20x-regulator", > + }, { > + .name = "axp20x-usb-power-supply", Could we use either "vbus-power-supply" to match the AXP datasheets, or "otg-power-supply" which is slightly more obvious to board owners? Thanks ChenYu > + .of_compatible = "x-powers,axp202-usb-power-supply", > + .num_resources = > + ARRAY_SIZE(axp20x_usb_power_supply_resources), > + .resources = axp20x_usb_power_supply_resources, > }, > }; > > -- > 2.3.6 > > -- > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 09 Jun 2015, Hans de Goede wrote: > Add a cell for the usb power_supply part of the axp20x PMICs. > > Note that this cell is only for the usb power_supply part and not the > ac-power / battery-charger / rtc-backup-bat-charger bits. > > Depending on the board each of those must be enabled / disabled separately > in devicetree as most boards do not use all 4. So in dt each one needs its > own child-node of the axp20x node. Another reason for using separate child > nodes for each is so that other devicetree nodes can have a power-supply > property with a phandle referencing a node representing a single > power-supply. > > The decision to use a separate devicetree node for each is reflected on > the kernel side by each getting its own mfd-cell / platform_device and > platform-driver. > > Cc: Bruno Prémont <bonbons@linux-vserver.org> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/mfd/axp20x.c | 32 +++++++++++++++++++++++++++++++- > 1 file changed, 31 insertions(+), 1 deletion(-) > > diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c > index 6ffbc11..47ce233 100644 > --- a/drivers/mfd/axp20x.c > +++ b/drivers/mfd/axp20x.c > @@ -113,6 +113,30 @@ static struct resource axp20x_pek_resources[] = { > }, > }; > > +static struct resource axp20x_usb_power_supply_resources[] = { > + { > + .name = "VBUS_PLUGIN", > + .start = AXP20X_IRQ_VBUS_PLUGIN, > + .end = AXP20X_IRQ_VBUS_PLUGIN, > + .flags = IORESOURCE_IRQ, > + }, { > + .name = "VBUS_REMOVAL", > + .start = AXP20X_IRQ_VBUS_REMOVAL, > + .end = AXP20X_IRQ_VBUS_REMOVAL, > + .flags = IORESOURCE_IRQ, > + }, { > + .name = "VBUS_VALID", > + .start = AXP20X_IRQ_VBUS_VALID, > + .end = AXP20X_IRQ_VBUS_VALID, > + .flags = IORESOURCE_IRQ, > + }, { > + .name = "VBUS_NOT_VALID", > + .start = AXP20X_IRQ_VBUS_NOT_VALID, > + .end = AXP20X_IRQ_VBUS_NOT_VALID, > + .flags = IORESOURCE_IRQ, > + }, > +}; Can you take a look at the DEFINE_RES_* macros in include/linux/ioport.h please? > static struct resource axp22x_pek_resources[] = { > { > .name = "PEK_DBR", > @@ -165,7 +189,7 @@ static const struct regmap_config axp20x_regmap_config = { > .val_bits = 8, > .wr_table = &axp20x_writeable_table, > .volatile_table = &axp20x_volatile_table, > - .max_register = AXP20X_FG_RES, > + .max_register = AXP20X_OCV(15), > .cache_type = REGCACHE_RBTREE, > }; > > @@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = { > .resources = axp20x_pek_resources, > }, { > .name = "axp20x-regulator", > + }, { > + .name = "axp20x-usb-power-supply", > + .of_compatible = "x-powers,axp202-usb-power-supply", > + .num_resources = > + ARRAY_SIZE(axp20x_usb_power_supply_resources), This wrap is only necessary due to the extreme tabbing used to line up the '='. Please do something about that instead. > + .resources = axp20x_usb_power_supply_resources, > }, > }; >
On Tue, 09 Jun 2015, Hans de Goede wrote: > Add a cell for the usb power_supply part of the axp20x PMICs. > > Note that this cell is only for the usb power_supply part and not the > ac-power / battery-charger / rtc-backup-bat-charger bits. > > Depending on the board each of those must be enabled / disabled separately > in devicetree as most boards do not use all 4. So in dt each one needs its > own child-node of the axp20x node. Another reason for using separate child > nodes for each is so that other devicetree nodes can have a power-supply > property with a phandle referencing a node representing a single > power-supply. > > The decision to use a separate devicetree node for each is reflected on > the kernel side by each getting its own mfd-cell / platform_device and > platform-driver. > > Cc: Bruno Prémont <bonbons@linux-vserver.org> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/mfd/axp20x.c | 32 +++++++++++++++++++++++++++++++- > 1 file changed, 31 insertions(+), 1 deletion(-) > > diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c > index 6ffbc11..47ce233 100644 > --- a/drivers/mfd/axp20x.c > +++ b/drivers/mfd/axp20x.c > @@ -113,6 +113,30 @@ static struct resource axp20x_pek_resources[] = { > }, > }; > > +static struct resource axp20x_usb_power_supply_resources[] = { > + { > + .name = "VBUS_PLUGIN", > + .start = AXP20X_IRQ_VBUS_PLUGIN, > + .end = AXP20X_IRQ_VBUS_PLUGIN, > + .flags = IORESOURCE_IRQ, > + }, { > + .name = "VBUS_REMOVAL", > + .start = AXP20X_IRQ_VBUS_REMOVAL, > + .end = AXP20X_IRQ_VBUS_REMOVAL, > + .flags = IORESOURCE_IRQ, > + }, { > + .name = "VBUS_VALID", > + .start = AXP20X_IRQ_VBUS_VALID, > + .end = AXP20X_IRQ_VBUS_VALID, > + .flags = IORESOURCE_IRQ, > + }, { > + .name = "VBUS_NOT_VALID", > + .start = AXP20X_IRQ_VBUS_NOT_VALID, > + .end = AXP20X_IRQ_VBUS_NOT_VALID, > + .flags = IORESOURCE_IRQ, > + }, > +}; > + > static struct resource axp22x_pek_resources[] = { > { > .name = "PEK_DBR", > @@ -165,7 +189,7 @@ static const struct regmap_config axp20x_regmap_config = { > .val_bits = 8, > .wr_table = &axp20x_writeable_table, > .volatile_table = &axp20x_volatile_table, > - .max_register = AXP20X_FG_RES, > + .max_register = AXP20X_OCV(15), Please define 15, as MAX_WHATEVER_REG or something. > .cache_type = REGCACHE_RBTREE, > }; > > @@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = { > .resources = axp20x_pek_resources, > }, { > .name = "axp20x-regulator", > + }, { > + .name = "axp20x-usb-power-supply", > + .of_compatible = "x-powers,axp202-usb-power-supply", > + .num_resources = > + ARRAY_SIZE(axp20x_usb_power_supply_resources), > + .resources = axp20x_usb_power_supply_resources, > }, > }; >
Hi, On 10-06-15 03:19, Chen-Yu Tsai wrote: > On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede <hdegoede@redhat.com> wrote: >> Add a cell for the usb power_supply part of the axp20x PMICs. >> >> Note that this cell is only for the usb power_supply part and not the >> ac-power / battery-charger / rtc-backup-bat-charger bits. >> >> Depending on the board each of those must be enabled / disabled separately >> in devicetree as most boards do not use all 4. So in dt each one needs its >> own child-node of the axp20x node. Another reason for using separate child >> nodes for each is so that other devicetree nodes can have a power-supply >> property with a phandle referencing a node representing a single >> power-supply. >> >> The decision to use a separate devicetree node for each is reflected on >> the kernel side by each getting its own mfd-cell / platform_device and >> platform-driver. >> >> Cc: Bruno Prémont <bonbons@linux-vserver.org> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/mfd/axp20x.c | 32 +++++++++++++++++++++++++++++++- >> 1 file changed, 31 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c >> index 6ffbc11..47ce233 100644 >> --- a/drivers/mfd/axp20x.c >> +++ b/drivers/mfd/axp20x.c >> @@ -113,6 +113,30 @@ static struct resource axp20x_pek_resources[] = { >> }, >> }; >> >> +static struct resource axp20x_usb_power_supply_resources[] = { >> + { >> + .name = "VBUS_PLUGIN", >> + .start = AXP20X_IRQ_VBUS_PLUGIN, >> + .end = AXP20X_IRQ_VBUS_PLUGIN, >> + .flags = IORESOURCE_IRQ, >> + }, { >> + .name = "VBUS_REMOVAL", >> + .start = AXP20X_IRQ_VBUS_REMOVAL, >> + .end = AXP20X_IRQ_VBUS_REMOVAL, >> + .flags = IORESOURCE_IRQ, >> + }, { >> + .name = "VBUS_VALID", >> + .start = AXP20X_IRQ_VBUS_VALID, >> + .end = AXP20X_IRQ_VBUS_VALID, >> + .flags = IORESOURCE_IRQ, >> + }, { >> + .name = "VBUS_NOT_VALID", >> + .start = AXP20X_IRQ_VBUS_NOT_VALID, >> + .end = AXP20X_IRQ_VBUS_NOT_VALID, >> + .flags = IORESOURCE_IRQ, >> + }, >> +}; >> + >> static struct resource axp22x_pek_resources[] = { >> { >> .name = "PEK_DBR", >> @@ -165,7 +189,7 @@ static const struct regmap_config axp20x_regmap_config = { >> .val_bits = 8, >> .wr_table = &axp20x_writeable_table, >> .volatile_table = &axp20x_volatile_table, >> - .max_register = AXP20X_FG_RES, >> + .max_register = AXP20X_OCV(15), >> .cache_type = REGCACHE_RBTREE, >> }; >> >> @@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = { >> .resources = axp20x_pek_resources, >> }, { >> .name = "axp20x-regulator", >> + }, { >> + .name = "axp20x-usb-power-supply", > > Could we use either "vbus-power-supply" to match the AXP datasheets, > or "otg-power-supply" which is slightly more obvious to board owners? I do not like the vbus name, since it does not indicate which bus it is, OTOH you are right that is what it is called in the datasheet. As for using otg, I think that usb is better then. All in all I believe that the current usb name is best, but if others disagree I'm open to renaming this. So anyone else have an opinion on what would be a good name for the cell and the compatible ? Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 10, 2015 at 09:57:13AM +0200, Hans de Goede wrote: > >>@@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = { > >> .resources = axp20x_pek_resources, > >> }, { > >> .name = "axp20x-regulator", > >>+ }, { > >>+ .name = "axp20x-usb-power-supply", > > > >Could we use either "vbus-power-supply" to match the AXP datasheets, > >or "otg-power-supply" which is slightly more obvious to board owners? > > I do not like the vbus name, since it does not indicate which bus > it is, OTOH you are right that is what it is called in the datasheet. > > As for using otg, I think that usb is better then. > > All in all I believe that the current usb name is best, but if others > disagree I'm open to renaming this. > > So anyone else have an opinion on what would be a good name for the > cell and the compatible ? I usually prefer to use the name mentionned in the datasheet, but if that doesn't make sense, feel free to use an alternative like this one. Maxime
Hello, On 13 June 2015 at 15:50, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Wed, Jun 10, 2015 at 09:57:13AM +0200, Hans de Goede wrote: >> >>@@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = { >> >> .resources = axp20x_pek_resources, >> >> }, { >> >> .name = "axp20x-regulator", >> >>+ }, { >> >>+ .name = "axp20x-usb-power-supply", >> > >> >Could we use either "vbus-power-supply" to match the AXP datasheets, >> >or "otg-power-supply" which is slightly more obvious to board owners? >> >> I do not like the vbus name, since it does not indicate which bus >> it is, OTOH you are right that is what it is called in the datasheet. >> >> As for using otg, I think that usb is better then. >> >> All in all I believe that the current usb name is best, but if others >> disagree I'm open to renaming this. >> >> So anyone else have an opinion on what would be a good name for the >> cell and the compatible ? > > I usually prefer to use the name mentionned in the datasheet, but if > that doesn't make sense, feel free to use an alternative like this > one. The power source is known as USB power since the reference Allwinner design connects this pin to the USB OTG connector and all known boards that use the AXP copy the design. If somebody gets creative with the vbus input or there is another manufacturer using AXP with different design it might get problematic naming it usb. However, either name is pretty much arbitrary and there is no need for the vbus to be connected to a bus connector. You could design a board with two plain power jacks and the AXP would switch between those according to its priority. If this priority is not configurable then I think it would be most descriptive to name the inputs as primary and secondary AC. The most important part is adding proper documentation so whatever the name the user can understand that this is the part that is called vbus in datasheed and is typically wired to the usb otg port. I only found very terse doc patch. It adds a new file without a reference in the main axp20x file. IMHO there should be a reference for the sub-nodes or they should be all in one file. Spreading the text into multitude of files that contain 2 lines of explanation and an example seems counter-productive. The examples can be combined in one. Another nit is that the doc shows axp202-usb-power-supply compatible while the usb power is available on all AXP regulators for which there are drivers since axp152. IMHO this can be just x-powers,usb-power-supply since it can only appear as subnode of the axp, anyway. Thanks Michal -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c index 6ffbc11..47ce233 100644 --- a/drivers/mfd/axp20x.c +++ b/drivers/mfd/axp20x.c @@ -113,6 +113,30 @@ static struct resource axp20x_pek_resources[] = { }, }; +static struct resource axp20x_usb_power_supply_resources[] = { + { + .name = "VBUS_PLUGIN", + .start = AXP20X_IRQ_VBUS_PLUGIN, + .end = AXP20X_IRQ_VBUS_PLUGIN, + .flags = IORESOURCE_IRQ, + }, { + .name = "VBUS_REMOVAL", + .start = AXP20X_IRQ_VBUS_REMOVAL, + .end = AXP20X_IRQ_VBUS_REMOVAL, + .flags = IORESOURCE_IRQ, + }, { + .name = "VBUS_VALID", + .start = AXP20X_IRQ_VBUS_VALID, + .end = AXP20X_IRQ_VBUS_VALID, + .flags = IORESOURCE_IRQ, + }, { + .name = "VBUS_NOT_VALID", + .start = AXP20X_IRQ_VBUS_NOT_VALID, + .end = AXP20X_IRQ_VBUS_NOT_VALID, + .flags = IORESOURCE_IRQ, + }, +}; + static struct resource axp22x_pek_resources[] = { { .name = "PEK_DBR", @@ -165,7 +189,7 @@ static const struct regmap_config axp20x_regmap_config = { .val_bits = 8, .wr_table = &axp20x_writeable_table, .volatile_table = &axp20x_volatile_table, - .max_register = AXP20X_FG_RES, + .max_register = AXP20X_OCV(15), .cache_type = REGCACHE_RBTREE, }; @@ -368,6 +392,12 @@ static struct mfd_cell axp20x_cells[] = { .resources = axp20x_pek_resources, }, { .name = "axp20x-regulator", + }, { + .name = "axp20x-usb-power-supply", + .of_compatible = "x-powers,axp202-usb-power-supply", + .num_resources = + ARRAY_SIZE(axp20x_usb_power_supply_resources), + .resources = axp20x_usb_power_supply_resources, }, };
Add a cell for the usb power_supply part of the axp20x PMICs. Note that this cell is only for the usb power_supply part and not the ac-power / battery-charger / rtc-backup-bat-charger bits. Depending on the board each of those must be enabled / disabled separately in devicetree as most boards do not use all 4. So in dt each one needs its own child-node of the axp20x node. Another reason for using separate child nodes for each is so that other devicetree nodes can have a power-supply property with a phandle referencing a node representing a single power-supply. The decision to use a separate devicetree node for each is reflected on the kernel side by each getting its own mfd-cell / platform_device and platform-driver. Cc: Bruno Prémont <bonbons@linux-vserver.org> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/mfd/axp20x.c | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-)