Re: [PATCH 04/11] net: phylink: switch to using fwnode_gpiod_get_index()
On Thu, Sep 12, 2019 at 04:44:29PM +0300, Andy Shevchenko wrote: > On Thu, Sep 12, 2019 at 10:41:43AM +0100, Linus Walleij wrote: > > On Wed, Sep 11, 2019 at 10:51 AM Dmitry Torokhov > > wrote: > > > > > If we are willing to sacrifice the custom label for the GPIO that > > > fwnode_gpiod_get_index() allows us to set, then there are several > > > drivers that could actually use gpiod_get() API. > > > > We have: > > gpiod_set_consumer_name(gpiod, "name"); > > to deal with that so no sacrifice is needed. > > Thank for this hint! Would it be possible to improve your email etiquette, and move this discussion to a more appropriate subject line, so I don't have to keep checking these emails, in case you _do_ talk about something relevent to the original patch that the subject line refers to? Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
Re: [PATCH 04/11] net: phylink: switch to using fwnode_gpiod_get_index()
On Thu, Sep 12, 2019 at 10:41:43AM +0100, Linus Walleij wrote: > On Wed, Sep 11, 2019 at 10:51 AM Dmitry Torokhov > wrote: > > > If we are willing to sacrifice the custom label for the GPIO that > > fwnode_gpiod_get_index() allows us to set, then there are several > > drivers that could actually use gpiod_get() API. > > We have: > gpiod_set_consumer_name(gpiod, "name"); > to deal with that so no sacrifice is needed. Thank for this hint! -- With Best Regards, Andy Shevchenko
Re: [PATCH 04/11] net: phylink: switch to using fwnode_gpiod_get_index()
On Wed, Sep 11, 2019 at 10:51 AM Dmitry Torokhov wrote: > If we are willing to sacrifice the custom label for the GPIO that > fwnode_gpiod_get_index() allows us to set, then there are several > drivers that could actually use gpiod_get() API. We have: gpiod_set_consumer_name(gpiod, "name"); to deal with that so no sacrifice is needed. Yours, Linus Walleij
Re: [PATCH 04/11] net: phylink: switch to using fwnode_gpiod_get_index()
On Wed, Sep 11, 2019 at 11:10:16AM +0100, Russell King - ARM Linux admin wrote: > On Wed, Sep 11, 2019 at 02:55:11AM -0700, Dmitry Torokhov wrote: > > On Wed, Sep 11, 2019 at 10:49:29AM +0100, Russell King - ARM Linux admin > > wrote: > > > On Wed, Sep 11, 2019 at 12:46:19PM +0300, Andy Shevchenko wrote: > > > > On Wed, Sep 11, 2019 at 10:39:14AM +0100, Russell King - ARM Linux > > > > admin wrote: > > > > > On Wed, Sep 11, 2019 at 12:25:14PM +0300, Andy Shevchenko wrote: > > > > > > On Wed, Sep 11, 2019 at 12:52:08AM -0700, Dmitry Torokhov wrote: > > > > > > > Instead of fwnode_get_named_gpiod() that I plan to hide away, > > > > > > > let's use > > > > > > > the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), > > > > > > > bit > > > > > > > works with arbitrary firmware node. > > > e > > > > > > > > I'm wondering if it's possible to step forward and replace > > > > > > fwnode_get_gpiod_index by gpiod_get() / gpiod_get_index() here and > > > > > > in other cases in this series. > > > > > > > > > > No, those require a struct device, but we have none. There are > > > > > network > > > > > drivers where there is a struct device for the network complex, but > > > > > only > > > > > DT nodes for the individual network interfaces. So no, gpiod_* really > > > > > doesn't work. > > > > > > > > In the following patch the node is derived from struct device. So, I > > > > believe > > > > some cases can be handled differently. > Referring back to my comment, notice that I said we have none for the > phylink case, so it's not possible there. > > I'm not sure why Andy replied the way he did, unless he mis-read my > comment. It is a first patch which does the change. Mostly my reply was to Dmitry and your comment clarifies the case with this patch, thanks! -- With Best Regards, Andy Shevchenko
Re: [PATCH 04/11] net: phylink: switch to using fwnode_gpiod_get_index()
On Wed, Sep 11, 2019 at 02:55:11AM -0700, Dmitry Torokhov wrote: > On Wed, Sep 11, 2019 at 10:49:29AM +0100, Russell King - ARM Linux admin > wrote: > > On Wed, Sep 11, 2019 at 12:46:19PM +0300, Andy Shevchenko wrote: > > > On Wed, Sep 11, 2019 at 10:39:14AM +0100, Russell King - ARM Linux admin > > > wrote: > > > > On Wed, Sep 11, 2019 at 12:25:14PM +0300, Andy Shevchenko wrote: > > > > > On Wed, Sep 11, 2019 at 12:52:08AM -0700, Dmitry Torokhov wrote: > > > > > > Instead of fwnode_get_named_gpiod() that I plan to hide away, let's > > > > > > use > > > > > > the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), bit > > > > > > works with arbitrary firmware node. > > e > > > > > > > I'm wondering if it's possible to step forward and replace > > > > > fwnode_get_gpiod_index by gpiod_get() / gpiod_get_index() here and > > > > > in other cases in this series. > > > > > > > > No, those require a struct device, but we have none. There are network > > > > drivers where there is a struct device for the network complex, but only > > > > DT nodes for the individual network interfaces. So no, gpiod_* really > > > > doesn't work. > > > > > > In the following patch the node is derived from struct device. So, I > > > believe > > > some cases can be handled differently. > > > > phylink is not passed a struct device - it has no knowledge what the > > parent device is. > > > > In any case, I do not have "the following patch". > > Andy is talking about this one: > > diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c > index ce940871331e..9ca51d678123 100644 > --- a/drivers/net/phy/mdio_bus.c > +++ b/drivers/net/phy/mdio_bus.c > @@ -46,8 +46,8 @@ static int mdiobus_register_gpiod(struct mdio_device > *mdiodev) > > /* Deassert the optional reset signal */ > if (mdiodev->dev.of_node) > - gpiod = fwnode_get_named_gpiod(&mdiodev->dev.of_node->fwnode, > - "reset-gpios", 0, >GPIOD_OUT_LOW, > + gpiod = fwnode_gpiod_get_index(&mdiodev->dev.of_node->fwnode, > + "reset", 0, GPIOD_OUT_LOW, >"PHY reset"); > Here if we do not care about "PHY reset" label, we could use > gpiod_get(&mdiodev->dev, "reset", GPIOD_OUT_LOW). Here, you have a struct device, so yes, it's possible. Referring back to my comment, notice that I said we have none for the phylink case, so it's not possible there. I'm not sure why Andy replied the way he did, unless he mis-read my comment. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
Re: [PATCH 04/11] net: phylink: switch to using fwnode_gpiod_get_index()
On Wed, Sep 11, 2019 at 10:49:29AM +0100, Russell King - ARM Linux admin wrote: > On Wed, Sep 11, 2019 at 12:46:19PM +0300, Andy Shevchenko wrote: > > On Wed, Sep 11, 2019 at 10:39:14AM +0100, Russell King - ARM Linux admin > > wrote: > > > On Wed, Sep 11, 2019 at 12:25:14PM +0300, Andy Shevchenko wrote: > > > > On Wed, Sep 11, 2019 at 12:52:08AM -0700, Dmitry Torokhov wrote: > > > > > Instead of fwnode_get_named_gpiod() that I plan to hide away, let's > > > > > use > > > > > the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), bit > > > > > works with arbitrary firmware node. > e > > > > > > I'm wondering if it's possible to step forward and replace > > > > fwnode_get_gpiod_index by gpiod_get() / gpiod_get_index() here and > > > > in other cases in this series. > > > > > > No, those require a struct device, but we have none. There are network > > > drivers where there is a struct device for the network complex, but only > > > DT nodes for the individual network interfaces. So no, gpiod_* really > > > doesn't work. > > > > In the following patch the node is derived from struct device. So, I believe > > some cases can be handled differently. > > phylink is not passed a struct device - it has no knowledge what the > parent device is. > > In any case, I do not have "the following patch". Andy is talking about this one: diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c index ce940871331e..9ca51d678123 100644 --- a/drivers/net/phy/mdio_bus.c +++ b/drivers/net/phy/mdio_bus.c @@ -46,8 +46,8 @@ static int mdiobus_register_gpiod(struct mdio_device *mdiodev) /* Deassert the optional reset signal */ if (mdiodev->dev.of_node) - gpiod = fwnode_get_named_gpiod(&mdiodev->dev.of_node->fwnode, - "reset-gpios", 0, GPIOD_OUT_LOW, + gpiod = fwnode_gpiod_get_index(&mdiodev->dev.of_node->fwnode, + "reset", 0, GPIOD_OUT_LOW, "PHY reset"); Here if we do not care about "PHY reset" label, we could use gpiod_get(&mdiodev->dev, "reset", GPIOD_OUT_LOW). Thanks. -- Dmitry
Re: [PATCH 04/11] net: phylink: switch to using fwnode_gpiod_get_index()
On Wed, Sep 11, 2019 at 12:46:19PM +0300, Andy Shevchenko wrote: > On Wed, Sep 11, 2019 at 10:39:14AM +0100, Russell King - ARM Linux admin > wrote: > > On Wed, Sep 11, 2019 at 12:25:14PM +0300, Andy Shevchenko wrote: > > > On Wed, Sep 11, 2019 at 12:52:08AM -0700, Dmitry Torokhov wrote: > > > > Instead of fwnode_get_named_gpiod() that I plan to hide away, let's use > > > > the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), bit > > > > works with arbitrary firmware node. > > > > > > I'm wondering if it's possible to step forward and replace > > > fwnode_get_gpiod_index by gpiod_get() / gpiod_get_index() here and > > > in other cases in this series. > > > > No, those require a struct device, but we have none. There are network > > drivers where there is a struct device for the network complex, but only > > DT nodes for the individual network interfaces. So no, gpiod_* really > > doesn't work. > > In the following patch the node is derived from struct device. So, I believe > some cases can be handled differently. If we are willing to sacrifice the custom label for the GPIO that fwnode_gpiod_get_index() allows us to set, then there are several drivers that could actually use gpiod_get() API. This is up to the dirver's maintainers... Thanks. -- Dmitry
Re: [PATCH 04/11] net: phylink: switch to using fwnode_gpiod_get_index()
On Wed, Sep 11, 2019 at 12:46:19PM +0300, Andy Shevchenko wrote: > On Wed, Sep 11, 2019 at 10:39:14AM +0100, Russell King - ARM Linux admin > wrote: > > On Wed, Sep 11, 2019 at 12:25:14PM +0300, Andy Shevchenko wrote: > > > On Wed, Sep 11, 2019 at 12:52:08AM -0700, Dmitry Torokhov wrote: > > > > Instead of fwnode_get_named_gpiod() that I plan to hide away, let's use > > > > the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), bit > > > > works with arbitrary firmware node. > > > > > > I'm wondering if it's possible to step forward and replace > > > fwnode_get_gpiod_index by gpiod_get() / gpiod_get_index() here and > > > in other cases in this series. > > > > No, those require a struct device, but we have none. There are network > > drivers where there is a struct device for the network complex, but only > > DT nodes for the individual network interfaces. So no, gpiod_* really > > doesn't work. > > In the following patch the node is derived from struct device. So, I believe > some cases can be handled differently. phylink is not passed a struct device - it has no knowledge what the parent device is. In any case, I do not have "the following patch". -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
Re: [PATCH 04/11] net: phylink: switch to using fwnode_gpiod_get_index()
On Wed, Sep 11, 2019 at 10:39:14AM +0100, Russell King - ARM Linux admin wrote: > On Wed, Sep 11, 2019 at 12:25:14PM +0300, Andy Shevchenko wrote: > > On Wed, Sep 11, 2019 at 12:52:08AM -0700, Dmitry Torokhov wrote: > > > Instead of fwnode_get_named_gpiod() that I plan to hide away, let's use > > > the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), bit > > > works with arbitrary firmware node. > > > > I'm wondering if it's possible to step forward and replace > > fwnode_get_gpiod_index by gpiod_get() / gpiod_get_index() here and > > in other cases in this series. > > No, those require a struct device, but we have none. There are network > drivers where there is a struct device for the network complex, but only > DT nodes for the individual network interfaces. So no, gpiod_* really > doesn't work. In the following patch the node is derived from struct device. So, I believe some cases can be handled differently. -- With Best Regards, Andy Shevchenko
Re: [PATCH 04/11] net: phylink: switch to using fwnode_gpiod_get_index()
On Wed, Sep 11, 2019 at 12:25:14PM +0300, Andy Shevchenko wrote: > On Wed, Sep 11, 2019 at 12:52:08AM -0700, Dmitry Torokhov wrote: > > Instead of fwnode_get_named_gpiod() that I plan to hide away, let's use > > the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), bit > > works with arbitrary firmware node. > > I'm wondering if it's possible to step forward and replace > fwnode_get_gpiod_index by gpiod_get() / gpiod_get_index() here and > in other cases in this series. No, those require a struct device, but we have none. There are network drivers where there is a struct device for the network complex, but only DT nodes for the individual network interfaces. So no, gpiod_* really doesn't work. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
Re: [PATCH 04/11] net: phylink: switch to using fwnode_gpiod_get_index()
On Wed, Sep 11, 2019 at 12:52:08AM -0700, Dmitry Torokhov wrote: > Instead of fwnode_get_named_gpiod() that I plan to hide away, let's use > the new fwnode_gpiod_get_index() that mimics gpiod_get_index(), bit > works with arbitrary firmware node. I'm wondering if it's possible to step forward and replace fwnode_get_gpiod_index by gpiod_get() / gpiod_get_index() here and in other cases in this series. -- With Best Regards, Andy Shevchenko
