| Message ID | 1390755901-3743-1-git-send-email-ben.dooks@codethink.co.uk (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi, On Sun, Jan 26, 2014 at 05:05:01PM +0000, Ben Dooks wrote: > Add support for the phy-rcar-gen2-usb driver to be probed from device tree. > > Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> > Reviewed-by: Ian Molton <ian.molton@codethink.co.uk> > --- > Fixes from v2: > - fix missing of_if patch > (I clearly should not be let near git-rebase when hungry) > > Fixes from v1: > - use of_property_reasd-bool() > - remove unused of_id variable > > Cc: linux-usb@vger.kernel.org > Cc: linux-sh@vger.kernel.org > Cc: Magnus Damm <magnus.damm@gmail.com> > Cc: Simon Horman <horms@verge.net.au> > Cc: devicetree@vger.kernel.org it's a good practice to Cc maintainers ;-) patch looks good to me, but I'd like to get an Ack from one of DT maintainers > --- > drivers/usb/phy/phy-rcar-gen2-usb.c | 31 ++++++++++++++++++++++++++----- > 1 file changed, 26 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/phy/phy-rcar-gen2-usb.c b/drivers/usb/phy/phy-rcar-gen2-usb.c > index db3ab34..e4665b9 100644 > --- a/drivers/usb/phy/phy-rcar-gen2-usb.c > +++ b/drivers/usb/phy/phy-rcar-gen2-usb.c > @@ -15,6 +15,7 @@ > #include <linux/module.h> > #include <linux/platform_data/usb-rcar-gen2-phy.h> > #include <linux/platform_device.h> > +#include <linux/of_device.h> > #include <linux/spinlock.h> > #include <linux/usb/otg.h> > > @@ -167,6 +168,12 @@ out: > spin_unlock_irqrestore(&priv->lock, flags); > } > > +static struct of_device_id rcar_gen2_usb_phy_ofmatch[] = { > + { .compatible = "renesas,usb-phy-r8a7790", }, > + { .compatible = "renesas,rcar-gen2-usb-phy", }, > + { }, > +}; > + > static int rcar_gen2_usb_phy_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > @@ -178,7 +185,7 @@ static int rcar_gen2_usb_phy_probe(struct platform_device *pdev) > int retval; > > pdata = dev_get_platdata(&pdev->dev); > - if (!pdata) { > + if (!pdata && !dev->of_node) { > dev_err(dev, "No platform data\n"); > return -EINVAL; > } > @@ -203,16 +210,29 @@ static int rcar_gen2_usb_phy_probe(struct platform_device *pdev) > spin_lock_init(&priv->lock); > priv->clk = clk; > priv->base = base; > - priv->ugctrl2 = pdata->chan0_pci ? > - USBHS_UGCTRL2_USB0_PCI : USBHS_UGCTRL2_USB0_HS; > - priv->ugctrl2 |= pdata->chan2_pci ? > - USBHS_UGCTRL2_USB2_PCI : USBHS_UGCTRL2_USB2_SS; > priv->phy.dev = dev; > priv->phy.label = dev_name(dev); > priv->phy.init = rcar_gen2_usb_phy_init; > priv->phy.shutdown = rcar_gen2_usb_phy_shutdown; > priv->phy.set_suspend = rcar_gen2_usb_phy_set_suspend; > > + if (dev->of_node) { > + if (of_property_read_bool(dev->of_node, "renesas,usb0-hs")) > + priv->ugctrl2 = USBHS_UGCTRL2_USB0_HS; > + else > + priv->ugctrl2 = USBHS_UGCTRL2_USB0_PCI; > + > + if (of_property_read_bool(dev->of_node, "renesas,usb2-ss")) > + priv->ugctrl2 |= USBHS_UGCTRL2_USB2_SS; > + else > + priv->ugctrl2 |= USBHS_UGCTRL2_USB2_PCI; > + } else { > + priv->ugctrl2 = pdata->chan0_pci ? > + USBHS_UGCTRL2_USB0_PCI : USBHS_UGCTRL2_USB0_HS; > + priv->ugctrl2 |= pdata->chan2_pci ? > + USBHS_UGCTRL2_USB2_PCI : USBHS_UGCTRL2_USB2_SS; > + } > + > retval = usb_add_phy(&priv->phy, USB_PHY_TYPE_USB2); > if (retval < 0) { > dev_err(dev, "Failed to add USB phy\n"); > @@ -236,6 +256,7 @@ static int rcar_gen2_usb_phy_remove(struct platform_device *pdev) > static struct platform_driver rcar_gen2_usb_phy_driver = { > .driver = { > .name = "usb_phy_rcar_gen2", > + .of_match_table = rcar_gen2_usb_phy_ofmatch, > }, > .probe = rcar_gen2_usb_phy_probe, > .remove = rcar_gen2_usb_phy_remove, > -- > 1.8.5.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 01/26/2014 08:05 PM, Ben Dooks wrote: > Add support for the phy-rcar-gen2-usb driver to be probed from device tree. > Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> > Reviewed-by: Ian Molton <ian.molton@codethink.co.uk> > --- > Fixes from v2: > - fix missing of_if patch > (I clearly should not be let near git-rebase when hungry) > Fixes from v1: > - use of_property_reasd-bool() > - remove unused of_id variable > Cc: linux-usb@vger.kernel.org > Cc: linux-sh@vger.kernel.org > Cc: Magnus Damm <magnus.damm@gmail.com> > Cc: Simon Horman <horms@verge.net.au> > Cc: devicetree@vger.kernel.org > --- > drivers/usb/phy/phy-rcar-gen2-usb.c | 31 ++++++++++++++++++++++++++----- > 1 file changed, 26 insertions(+), 5 deletions(-) > diff --git a/drivers/usb/phy/phy-rcar-gen2-usb.c b/drivers/usb/phy/phy-rcar-gen2-usb.c > index db3ab34..e4665b9 100644 > --- a/drivers/usb/phy/phy-rcar-gen2-usb.c > +++ b/drivers/usb/phy/phy-rcar-gen2-usb.c [...] > @@ -167,6 +168,12 @@ out: > spin_unlock_irqrestore(&priv->lock, flags); > } > > +static struct of_device_id rcar_gen2_usb_phy_ofmatch[] = { > + { .compatible = "renesas,usb-phy-r8a7790", }, > + { .compatible = "renesas,rcar-gen2-usb-phy", }, Frankly speaking, I don't understand the need for the clearly duplicate entries. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 27/01/14 18:23, Sergei Shtylyov wrote: > Hello. > > On 01/26/2014 08:05 PM, Ben Dooks wrote: [snip] >> >> +static struct of_device_id rcar_gen2_usb_phy_ofmatch[] = { >> + { .compatible = "renesas,usb-phy-r8a7790", }, >> + { .compatible = "renesas,rcar-gen2-usb-phy", }, > > Frankly speaking, I don't understand the need for the clearly > duplicate entries. Thanks, will look into remove it. Anyone else have any comments on this?
On Mon, Jan 27, 2014 at 07:06:26PM +0000, Ben Dooks wrote: > On 27/01/14 18:23, Sergei Shtylyov wrote: > >Hello. > > > >On 01/26/2014 08:05 PM, Ben Dooks wrote: > > [snip] > > >> > >>+static struct of_device_id rcar_gen2_usb_phy_ofmatch[] = { > >>+ { .compatible = "renesas,usb-phy-r8a7790", }, > >>+ { .compatible = "renesas,rcar-gen2-usb-phy", }, > > > > Frankly speaking, I don't understand the need for the clearly > >duplicate entries. > > Thanks, will look into remove it. > Anyone else have any comments on this? I would like you to leave it there. As we know the r8a7790 is an R-Car Gen2 SoC. But there are other R-Car Gen2 SoCs, such as the r8a7791, they it could plausibly make use of rcar-gen2-usb-phy until the driver is updated with a usb-phy-r8a7791 entry. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 29-01-2014 10:22, Simon Horman wrote: >> [snip] >>>> +static struct of_device_id rcar_gen2_usb_phy_ofmatch[] = { >>>> + { .compatible = "renesas,usb-phy-r8a7790", }, >>>> + { .compatible = "renesas,rcar-gen2-usb-phy", }, >>> Frankly speaking, I don't understand the need for the clearly >>> duplicate entries. >> Thanks, will look into remove it. >> Anyone else have any comments on this? > I would like you to leave it there. > As we know the r8a7790 is an R-Car Gen2 SoC. But there are other R-Car > Gen2 SoCs, such as the r8a7791, they it could plausibly make use of > rcar-gen2-usb-phy until the driver is updated with a usb-phy-r8a7791 entry. Why not just "update" the driver this way now, may I ask? WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 29, 2014 at 02:41:58PM +0400, Sergei Shtylyov wrote: > Hello. > > On 29-01-2014 10:22, Simon Horman wrote: > > >>[snip] > > >>>>+static struct of_device_id rcar_gen2_usb_phy_ofmatch[] = { > >>>>+ { .compatible = "renesas,usb-phy-r8a7790", }, > >>>>+ { .compatible = "renesas,rcar-gen2-usb-phy", }, > > >>> Frankly speaking, I don't understand the need for the clearly > >>>duplicate entries. > > >>Thanks, will look into remove it. > >>Anyone else have any comments on this? > > >I would like you to leave it there. > > >As we know the r8a7790 is an R-Car Gen2 SoC. But there are other R-Car > >Gen2 SoCs, such as the r8a7791, they it could plausibly make use of > >rcar-gen2-usb-phy until the driver is updated with a usb-phy-r8a7791 entry. > > Why not just "update" the driver this way now, may I ask? Because I don't believe that Ben has access to r8a7791 hardware to test the change and thus it makes sense to handle it separately. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 29-01-2014 16:22, Simon Horman wrote: >>>> [snip] >>>>>> +static struct of_device_id rcar_gen2_usb_phy_ofmatch[] = { >>>>>> + { .compatible = "renesas,usb-phy-r8a7790", }, >>>>>> + { .compatible = "renesas,rcar-gen2-usb-phy", }, >>>>> Frankly speaking, I don't understand the need for the clearly >>>>> duplicate entries. >>>> Thanks, will look into remove it. >>>> Anyone else have any comments on this? >>> I would like you to leave it there. >>> As we know the r8a7790 is an R-Car Gen2 SoC. But there are other R-Car >>> Gen2 SoCs, such as the r8a7791, they it could plausibly make use of >>> rcar-gen2-usb-phy until the driver is updated with a usb-phy-r8a7791 entry. >> Why not just "update" the driver this way now, may I ask? > Because I don't believe that Ben has access to r8a7791 hardware > to test the change and thus it makes sense to handle it separately. I believe Valentine has already tested the driver with both SoCs. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 29, 2014 at 04:26:55PM +0400, Sergei Shtylyov wrote: > Hello. > > On 29-01-2014 16:22, Simon Horman wrote: > > >>>>[snip] > > >>>>>>+static struct of_device_id rcar_gen2_usb_phy_ofmatch[] = { > >>>>>>+ { .compatible = "renesas,usb-phy-r8a7790", }, > >>>>>>+ { .compatible = "renesas,rcar-gen2-usb-phy", }, > > >>>>> Frankly speaking, I don't understand the need for the clearly > >>>>>duplicate entries. > > >>>>Thanks, will look into remove it. > >>>>Anyone else have any comments on this? > > >>>I would like you to leave it there. > > >>>As we know the r8a7790 is an R-Car Gen2 SoC. But there are other R-Car > >>>Gen2 SoCs, such as the r8a7791, they it could plausibly make use of > >>>rcar-gen2-usb-phy until the driver is updated with a usb-phy-r8a7791 entry. > > >> Why not just "update" the driver this way now, may I ask? > > >Because I don't believe that Ben has access to r8a7791 hardware > >to test the change and thus it makes sense to handle it separately. > > I believe Valentine has already tested the driver with both SoCs. Oh, great. Then I guess the compat string for r8a7791 can be added as you suggest. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" 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/usb/phy/phy-rcar-gen2-usb.c b/drivers/usb/phy/phy-rcar-gen2-usb.c index db3ab34..e4665b9 100644 --- a/drivers/usb/phy/phy-rcar-gen2-usb.c +++ b/drivers/usb/phy/phy-rcar-gen2-usb.c @@ -15,6 +15,7 @@ #include <linux/module.h> #include <linux/platform_data/usb-rcar-gen2-phy.h> #include <linux/platform_device.h> +#include <linux/of_device.h> #include <linux/spinlock.h> #include <linux/usb/otg.h> @@ -167,6 +168,12 @@ out: spin_unlock_irqrestore(&priv->lock, flags); } +static struct of_device_id rcar_gen2_usb_phy_ofmatch[] = { + { .compatible = "renesas,usb-phy-r8a7790", }, + { .compatible = "renesas,rcar-gen2-usb-phy", }, + { }, +}; + static int rcar_gen2_usb_phy_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -178,7 +185,7 @@ static int rcar_gen2_usb_phy_probe(struct platform_device *pdev) int retval; pdata = dev_get_platdata(&pdev->dev); - if (!pdata) { + if (!pdata && !dev->of_node) { dev_err(dev, "No platform data\n"); return -EINVAL; } @@ -203,16 +210,29 @@ static int rcar_gen2_usb_phy_probe(struct platform_device *pdev) spin_lock_init(&priv->lock); priv->clk = clk; priv->base = base; - priv->ugctrl2 = pdata->chan0_pci ? - USBHS_UGCTRL2_USB0_PCI : USBHS_UGCTRL2_USB0_HS; - priv->ugctrl2 |= pdata->chan2_pci ? - USBHS_UGCTRL2_USB2_PCI : USBHS_UGCTRL2_USB2_SS; priv->phy.dev = dev; priv->phy.label = dev_name(dev); priv->phy.init = rcar_gen2_usb_phy_init; priv->phy.shutdown = rcar_gen2_usb_phy_shutdown; priv->phy.set_suspend = rcar_gen2_usb_phy_set_suspend; + if (dev->of_node) { + if (of_property_read_bool(dev->of_node, "renesas,usb0-hs")) + priv->ugctrl2 = USBHS_UGCTRL2_USB0_HS; + else + priv->ugctrl2 = USBHS_UGCTRL2_USB0_PCI; + + if (of_property_read_bool(dev->of_node, "renesas,usb2-ss")) + priv->ugctrl2 |= USBHS_UGCTRL2_USB2_SS; + else + priv->ugctrl2 |= USBHS_UGCTRL2_USB2_PCI; + } else { + priv->ugctrl2 = pdata->chan0_pci ? + USBHS_UGCTRL2_USB0_PCI : USBHS_UGCTRL2_USB0_HS; + priv->ugctrl2 |= pdata->chan2_pci ? + USBHS_UGCTRL2_USB2_PCI : USBHS_UGCTRL2_USB2_SS; + } + retval = usb_add_phy(&priv->phy, USB_PHY_TYPE_USB2); if (retval < 0) { dev_err(dev, "Failed to add USB phy\n"); @@ -236,6 +256,7 @@ static int rcar_gen2_usb_phy_remove(struct platform_device *pdev) static struct platform_driver rcar_gen2_usb_phy_driver = { .driver = { .name = "usb_phy_rcar_gen2", + .of_match_table = rcar_gen2_usb_phy_ofmatch, }, .probe = rcar_gen2_usb_phy_probe, .remove = rcar_gen2_usb_phy_remove,