| Message ID | 1359993540-20780-2-git-send-email-rogerq@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Mon, Feb 04, 2013 at 05:58:48PM +0200, Roger Quadros wrote: > The PHY clock, clock rate, VCC regulator and RESET regulator > can now be provided via device tree. > > Signed-off-by: Roger Quadros <rogerq@ti.com> > --- > .../devicetree/bindings/usb/usb-nop-xceiv.txt | 34 ++++++++++++++++++++ > drivers/usb/otg/nop-usb-xceiv.c | 31 ++++++++++++++++++ > 2 files changed, 65 insertions(+), 0 deletions(-) > create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt > > diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt > new file mode 100644 > index 0000000..d7e2726 > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt > @@ -0,0 +1,34 @@ > +USB NOP PHY > + > +Required properties: > +- compatible: should be usb-nop-xceiv > + > +Optional properties: > +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree > + /bindings/clock/clock-bindings.txt > + This property is required if clock-frequency is specified. > + > +- clock-names: Should be "main_clk" > + > +- clock-frequency: the clock frequency (in Hz) that the PHY clock must > + be configured to. > + > +- vcc-supply: phandle to the regulator that provides RESET to the PHY. > + > +- reset-supply: phandle to the regulator that provides power to the PHY. > + > +Example: > + > + hsusb1_phy { > + compatible = "usb-nop-xceiv"; > + clock-frequency = <19200000>; > + clocks = <&osc 0>; > + clock-names = "main_clk"; > + vcc-supply = <&hsusb1_vcc_regulator>; > + reset-supply = <&hsusb1_reset_regulator>; > + }; > + > +hsusb1_phy is a NOP USB PHY device that gets its clock from an oscillator > +and expects that clock to be configured to 19.2MHz by the NOP PHY driver. > +hsusb1_vcc_regulator provides power to the PHY and hsusb1_reset_regulator > +controls RESET. > diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c > index ac027a1..adbb7ab 100644 > --- a/drivers/usb/otg/nop-usb-xceiv.c > +++ b/drivers/usb/otg/nop-usb-xceiv.c > @@ -34,6 +34,7 @@ > #include <linux/slab.h> > #include <linux/clk.h> > #include <linux/regulator/consumer.h> > +#include <linux/of.h> > > struct nop_usb_xceiv { > struct usb_phy phy; > @@ -138,8 +139,19 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host) > return 0; > } > > +static void nop_xeiv_get_dt_pdata(struct device *dev, asking to remove, but xeiv != xceiv :-) > + struct nop_usb_xceiv_platform_data *pdata) > +{ > + struct device_node *node = dev->of_node; > + u32 clk_rate; > + > + if (!of_property_read_u32(node, "clock-frequency", &clk_rate)) > + pdata->clk_rate = clk_rate; > +} > + > static int nop_usb_xceiv_probe(struct platform_device *pdev) > { > + struct device *dev = &pdev->dev; > struct nop_usb_xceiv_platform_data *pdata = pdev->dev.platform_data; > struct nop_usb_xceiv *nop; > enum usb_phy_type type = USB_PHY_TYPE_USB2; > @@ -153,6 +165,17 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev) > if (!nop->phy.otg) > return -ENOMEM; > > + if (dev->of_node) { > + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); > + if (!pdata) { > + dev_err(dev, "Memory allocation failure\n"); > + return -ENOMEM; > + } > + nop_xeiv_get_dt_pdata(dev, pdata); actually, I would prefer to not create pdata at all. I mean, ideally pdata would be used to initialize fields in your own structure, so first move clk_rate to your own private structure, copy pdata's clk_rate value to that, then you don't need this hackery when using DT.
On 02/05/2013 09:26 AM, Felipe Balbi wrote: > Hi, > > On Mon, Feb 04, 2013 at 05:58:48PM +0200, Roger Quadros wrote: >> The PHY clock, clock rate, VCC regulator and RESET regulator >> can now be provided via device tree. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> .../devicetree/bindings/usb/usb-nop-xceiv.txt | 34 ++++++++++++++++++++ >> drivers/usb/otg/nop-usb-xceiv.c | 31 ++++++++++++++++++ >> 2 files changed, 65 insertions(+), 0 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >> >> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >> new file mode 100644 >> index 0000000..d7e2726 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >> @@ -0,0 +1,34 @@ >> +USB NOP PHY >> + >> +Required properties: >> +- compatible: should be usb-nop-xceiv >> + >> +Optional properties: >> +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree >> + /bindings/clock/clock-bindings.txt >> + This property is required if clock-frequency is specified. >> + >> +- clock-names: Should be "main_clk" >> + >> +- clock-frequency: the clock frequency (in Hz) that the PHY clock must >> + be configured to. >> + >> +- vcc-supply: phandle to the regulator that provides RESET to the PHY. >> + >> +- reset-supply: phandle to the regulator that provides power to the PHY. >> + >> +Example: >> + >> + hsusb1_phy { >> + compatible = "usb-nop-xceiv"; >> + clock-frequency = <19200000>; >> + clocks = <&osc 0>; >> + clock-names = "main_clk"; >> + vcc-supply = <&hsusb1_vcc_regulator>; >> + reset-supply = <&hsusb1_reset_regulator>; >> + }; >> + >> +hsusb1_phy is a NOP USB PHY device that gets its clock from an oscillator >> +and expects that clock to be configured to 19.2MHz by the NOP PHY driver. >> +hsusb1_vcc_regulator provides power to the PHY and hsusb1_reset_regulator >> +controls RESET. >> diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c >> index ac027a1..adbb7ab 100644 >> --- a/drivers/usb/otg/nop-usb-xceiv.c >> +++ b/drivers/usb/otg/nop-usb-xceiv.c >> @@ -34,6 +34,7 @@ >> #include <linux/slab.h> >> #include <linux/clk.h> >> #include <linux/regulator/consumer.h> >> +#include <linux/of.h> >> >> struct nop_usb_xceiv { >> struct usb_phy phy; >> @@ -138,8 +139,19 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host) >> return 0; >> } >> >> +static void nop_xeiv_get_dt_pdata(struct device *dev, > > asking to remove, but xeiv != xceiv :-) > >> + struct nop_usb_xceiv_platform_data *pdata) >> +{ >> + struct device_node *node = dev->of_node; >> + u32 clk_rate; >> + >> + if (!of_property_read_u32(node, "clock-frequency", &clk_rate)) >> + pdata->clk_rate = clk_rate; >> +} >> + >> static int nop_usb_xceiv_probe(struct platform_device *pdev) >> { >> + struct device *dev = &pdev->dev; >> struct nop_usb_xceiv_platform_data *pdata = pdev->dev.platform_data; >> struct nop_usb_xceiv *nop; >> enum usb_phy_type type = USB_PHY_TYPE_USB2; >> @@ -153,6 +165,17 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev) >> if (!nop->phy.otg) >> return -ENOMEM; >> >> + if (dev->of_node) { >> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); >> + if (!pdata) { >> + dev_err(dev, "Memory allocation failure\n"); >> + return -ENOMEM; >> + } >> + nop_xeiv_get_dt_pdata(dev, pdata); > > actually, I would prefer to not create pdata at all. I mean, ideally > pdata would be used to initialize fields in your own structure, so first > move clk_rate to your own private structure, copy pdata's clk_rate value > to that, then you don't need this hackery when using DT. > OK, got it. Will revise. cheers, -roger
On Tue, Feb 05, 2013 at 10:30:49AM +0200, Roger Quadros wrote: > On 02/05/2013 09:26 AM, Felipe Balbi wrote: > > Hi, > > > > On Mon, Feb 04, 2013 at 05:58:48PM +0200, Roger Quadros wrote: > >> The PHY clock, clock rate, VCC regulator and RESET regulator > >> can now be provided via device tree. > >> > >> Signed-off-by: Roger Quadros <rogerq@ti.com> > >> --- > >> .../devicetree/bindings/usb/usb-nop-xceiv.txt | 34 ++++++++++++++++++++ > >> drivers/usb/otg/nop-usb-xceiv.c | 31 ++++++++++++++++++ > >> 2 files changed, 65 insertions(+), 0 deletions(-) > >> create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt > >> > >> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt > >> new file mode 100644 > >> index 0000000..d7e2726 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt > >> @@ -0,0 +1,34 @@ > >> +USB NOP PHY > >> + > >> +Required properties: > >> +- compatible: should be usb-nop-xceiv > >> + > >> +Optional properties: > >> +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree > >> + /bindings/clock/clock-bindings.txt > >> + This property is required if clock-frequency is specified. > >> + > >> +- clock-names: Should be "main_clk" > >> + > >> +- clock-frequency: the clock frequency (in Hz) that the PHY clock must > >> + be configured to. > >> + > >> +- vcc-supply: phandle to the regulator that provides RESET to the PHY. > >> + > >> +- reset-supply: phandle to the regulator that provides power to the PHY. > >> + > >> +Example: > >> + > >> + hsusb1_phy { > >> + compatible = "usb-nop-xceiv"; > >> + clock-frequency = <19200000>; > >> + clocks = <&osc 0>; > >> + clock-names = "main_clk"; > >> + vcc-supply = <&hsusb1_vcc_regulator>; > >> + reset-supply = <&hsusb1_reset_regulator>; > >> + }; > >> + > >> +hsusb1_phy is a NOP USB PHY device that gets its clock from an oscillator > >> +and expects that clock to be configured to 19.2MHz by the NOP PHY driver. > >> +hsusb1_vcc_regulator provides power to the PHY and hsusb1_reset_regulator > >> +controls RESET. > >> diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c > >> index ac027a1..adbb7ab 100644 > >> --- a/drivers/usb/otg/nop-usb-xceiv.c > >> +++ b/drivers/usb/otg/nop-usb-xceiv.c > >> @@ -34,6 +34,7 @@ > >> #include <linux/slab.h> > >> #include <linux/clk.h> > >> #include <linux/regulator/consumer.h> > >> +#include <linux/of.h> > >> > >> struct nop_usb_xceiv { > >> struct usb_phy phy; > >> @@ -138,8 +139,19 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host) > >> return 0; > >> } > >> > >> +static void nop_xeiv_get_dt_pdata(struct device *dev, > > > > asking to remove, but xeiv != xceiv :-) > > > >> + struct nop_usb_xceiv_platform_data *pdata) > >> +{ > >> + struct device_node *node = dev->of_node; > >> + u32 clk_rate; > >> + > >> + if (!of_property_read_u32(node, "clock-frequency", &clk_rate)) > >> + pdata->clk_rate = clk_rate; > >> +} > >> + > >> static int nop_usb_xceiv_probe(struct platform_device *pdev) > >> { > >> + struct device *dev = &pdev->dev; > >> struct nop_usb_xceiv_platform_data *pdata = pdev->dev.platform_data; > >> struct nop_usb_xceiv *nop; > >> enum usb_phy_type type = USB_PHY_TYPE_USB2; > >> @@ -153,6 +165,17 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev) > >> if (!nop->phy.otg) > >> return -ENOMEM; > >> > >> + if (dev->of_node) { > >> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); > >> + if (!pdata) { > >> + dev_err(dev, "Memory allocation failure\n"); > >> + return -ENOMEM; > >> + } > >> + nop_xeiv_get_dt_pdata(dev, pdata); > > > > actually, I would prefer to not create pdata at all. I mean, ideally > > pdata would be used to initialize fields in your own structure, so first > > move clk_rate to your own private structure, copy pdata's clk_rate value > > to that, then you don't need this hackery when using DT. > > > OK, got it. Will revise. Cool, after that you can add my: Acked-by: Felipe Balbi <balbi@ti.com>
On 02/04/2013 04:58 PM, Roger Quadros wrote: > The PHY clock, clock rate, VCC regulator and RESET regulator > can now be provided via device tree. > > Signed-off-by: Roger Quadros <rogerq@ti.com> > --- > .../devicetree/bindings/usb/usb-nop-xceiv.txt | 34 ++++++++++++++++++++ > drivers/usb/otg/nop-usb-xceiv.c | 31 ++++++++++++++++++ > 2 files changed, 65 insertions(+), 0 deletions(-) > create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt > > diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt > new file mode 100644 > index 0000000..d7e2726 > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt > @@ -0,0 +1,34 @@ > +USB NOP PHY > + > +Required properties: > +- compatible: should be usb-nop-xceiv > + > +Optional properties: > +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree > + /bindings/clock/clock-bindings.txt > + This property is required if clock-frequency is specified. > + > +- clock-names: Should be "main_clk" > + > +- clock-frequency: the clock frequency (in Hz) that the PHY clock must > + be configured to. > + > +- vcc-supply: phandle to the regulator that provides RESET to the PHY. > + > +- reset-supply: phandle to the regulator that provides power to the PHY. > + > +Example: > + > + hsusb1_phy { > + compatible = "usb-nop-xceiv"; > + clock-frequency = <19200000>; Why do you hardcode the clock frequency here? You should use clk_get_rate() to get the frequency from the clock tree. Marc
On 03/08/2013 12:46 PM, Marc Kleine-Budde wrote: > On 02/04/2013 04:58 PM, Roger Quadros wrote: >> The PHY clock, clock rate, VCC regulator and RESET regulator >> can now be provided via device tree. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> .../devicetree/bindings/usb/usb-nop-xceiv.txt | 34 ++++++++++++++++++++ >> drivers/usb/otg/nop-usb-xceiv.c | 31 ++++++++++++++++++ >> 2 files changed, 65 insertions(+), 0 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >> >> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >> new file mode 100644 >> index 0000000..d7e2726 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >> @@ -0,0 +1,34 @@ >> +USB NOP PHY >> + >> +Required properties: >> +- compatible: should be usb-nop-xceiv >> + >> +Optional properties: >> +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree >> + /bindings/clock/clock-bindings.txt >> + This property is required if clock-frequency is specified. >> + >> +- clock-names: Should be "main_clk" >> + >> +- clock-frequency: the clock frequency (in Hz) that the PHY clock must >> + be configured to. >> + >> +- vcc-supply: phandle to the regulator that provides RESET to the PHY. >> + >> +- reset-supply: phandle to the regulator that provides power to the PHY. >> + >> +Example: >> + >> + hsusb1_phy { >> + compatible = "usb-nop-xceiv"; >> + clock-frequency = <19200000>; > > Why do you hardcode the clock frequency here? You should use > clk_get_rate() to get the frequency from the clock tree. That would work only if the clock was programmed to the correct frequency by someone. e.g. In the OMAP case nobody programs the auxiliary clock on Panda which clocks the USB PHY. The usb-nop-xceiv device driver must program the clock rate using clk_set_rate(), but it needs to know what frequency it must program it to. Different boards/PHYs might use a different clock frequency. The 'clock-frequency' property is used to pass on this information to the driver. cheers, -roger
On 03/08/2013 11:46 AM, Marc Kleine-Budde wrote: > On 02/04/2013 04:58 PM, Roger Quadros wrote: >> The PHY clock, clock rate, VCC regulator and RESET regulator >> can now be provided via device tree. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> .../devicetree/bindings/usb/usb-nop-xceiv.txt | 34 ++++++++++++++++++++ >> drivers/usb/otg/nop-usb-xceiv.c | 31 ++++++++++++++++++ >> 2 files changed, 65 insertions(+), 0 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >> >> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >> new file mode 100644 >> index 0000000..d7e2726 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >> @@ -0,0 +1,34 @@ >> +USB NOP PHY >> + >> +Required properties: >> +- compatible: should be usb-nop-xceiv >> + >> +Optional properties: >> +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree >> + /bindings/clock/clock-bindings.txt >> + This property is required if clock-frequency is specified. >> + >> +- clock-names: Should be "main_clk" >> + >> +- clock-frequency: the clock frequency (in Hz) that the PHY clock must >> + be configured to. >> + >> +- vcc-supply: phandle to the regulator that provides RESET to the PHY. >> + >> +- reset-supply: phandle to the regulator that provides power to the PHY. >> + >> +Example: >> + >> + hsusb1_phy { >> + compatible = "usb-nop-xceiv"; >> + clock-frequency = <19200000>; > > Why do you hardcode the clock frequency here? You should use > clk_get_rate() to get the frequency from the clock tree. What about declaring a "fixed-clock" node in the device tree? Then it should be possible to keep the driver free of any omap specific code. Marc
On 03/08/2013 05:45 PM, Marc Kleine-Budde wrote: > On 03/08/2013 11:46 AM, Marc Kleine-Budde wrote: >> On 02/04/2013 04:58 PM, Roger Quadros wrote: >>> The PHY clock, clock rate, VCC regulator and RESET regulator >>> can now be provided via device tree. >>> >>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>> --- >>> .../devicetree/bindings/usb/usb-nop-xceiv.txt | 34 ++++++++++++++++++++ >>> drivers/usb/otg/nop-usb-xceiv.c | 31 ++++++++++++++++++ >>> 2 files changed, 65 insertions(+), 0 deletions(-) >>> create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >>> >>> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >>> new file mode 100644 >>> index 0000000..d7e2726 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >>> @@ -0,0 +1,34 @@ >>> +USB NOP PHY >>> + >>> +Required properties: >>> +- compatible: should be usb-nop-xceiv >>> + >>> +Optional properties: >>> +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree >>> + /bindings/clock/clock-bindings.txt >>> + This property is required if clock-frequency is specified. >>> + >>> +- clock-names: Should be "main_clk" >>> + >>> +- clock-frequency: the clock frequency (in Hz) that the PHY clock must >>> + be configured to. >>> + >>> +- vcc-supply: phandle to the regulator that provides RESET to the PHY. >>> + >>> +- reset-supply: phandle to the regulator that provides power to the PHY. >>> + >>> +Example: >>> + >>> + hsusb1_phy { >>> + compatible = "usb-nop-xceiv"; >>> + clock-frequency = <19200000>; >> >> Why do you hardcode the clock frequency here? You should use >> clk_get_rate() to get the frequency from the clock tree. > > What about declaring a "fixed-clock" node in the device tree? Then it > should be possible to keep the driver free of any omap specific code. > The current implementation is not OMAP specific and is not limited to a fixed frequency clock. The PHY driver is using standard clock APIs to set the clock rate 'only' if the 'clock-frequency' property is present in the device tree node. What is the benefit of declaring it as a "fixed-clock"? FYI. The clock may not necessarily be a fixed frequency clock and someone needs to program the rate. cheers, -roger
On 02/05/2013 08:26 AM, Felipe Balbi wrote: > Hi, > > On Mon, Feb 04, 2013 at 05:58:48PM +0200, Roger Quadros wrote: >> The PHY clock, clock rate, VCC regulator and RESET regulator >> can now be provided via device tree. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> .../devicetree/bindings/usb/usb-nop-xceiv.txt | 34 ++++++++++++++++++++ >> drivers/usb/otg/nop-usb-xceiv.c | 31 ++++++++++++++++++ >> 2 files changed, 65 insertions(+), 0 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >> >> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >> new file mode 100644 >> index 0000000..d7e2726 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >> @@ -0,0 +1,34 @@ >> +USB NOP PHY >> + >> +Required properties: >> +- compatible: should be usb-nop-xceiv >> + >> +Optional properties: >> +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree >> + /bindings/clock/clock-bindings.txt >> + This property is required if clock-frequency is specified. >> + >> +- clock-names: Should be "main_clk" >> + >> +- clock-frequency: the clock frequency (in Hz) that the PHY clock must >> + be configured to. >> + >> +- vcc-supply: phandle to the regulator that provides RESET to the PHY. >> + >> +- reset-supply: phandle to the regulator that provides power to the PHY. >> + >> +Example: >> + >> + hsusb1_phy { >> + compatible = "usb-nop-xceiv"; >> + clock-frequency = <19200000>; >> + clocks = <&osc 0>; >> + clock-names = "main_clk"; >> + vcc-supply = <&hsusb1_vcc_regulator>; >> + reset-supply = <&hsusb1_reset_regulator>; >> + }; >> + >> +hsusb1_phy is a NOP USB PHY device that gets its clock from an oscillator >> +and expects that clock to be configured to 19.2MHz by the NOP PHY driver. >> +hsusb1_vcc_regulator provides power to the PHY and hsusb1_reset_regulator >> +controls RESET. >> diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c >> index ac027a1..adbb7ab 100644 >> --- a/drivers/usb/otg/nop-usb-xceiv.c >> +++ b/drivers/usb/otg/nop-usb-xceiv.c >> @@ -34,6 +34,7 @@ >> #include <linux/slab.h> >> #include <linux/clk.h> >> #include <linux/regulator/consumer.h> >> +#include <linux/of.h> >> >> struct nop_usb_xceiv { >> struct usb_phy phy; >> @@ -138,8 +139,19 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host) >> return 0; >> } >> >> +static void nop_xeiv_get_dt_pdata(struct device *dev, > > asking to remove, but xeiv != xceiv :-) > >> + struct nop_usb_xceiv_platform_data *pdata) >> +{ >> + struct device_node *node = dev->of_node; >> + u32 clk_rate; >> + >> + if (!of_property_read_u32(node, "clock-frequency", &clk_rate)) >> + pdata->clk_rate = clk_rate; >> +} >> + >> static int nop_usb_xceiv_probe(struct platform_device *pdev) >> { >> + struct device *dev = &pdev->dev; >> struct nop_usb_xceiv_platform_data *pdata = pdev->dev.platform_data; >> struct nop_usb_xceiv *nop; >> enum usb_phy_type type = USB_PHY_TYPE_USB2; >> @@ -153,6 +165,17 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev) >> if (!nop->phy.otg) >> return -ENOMEM; >> >> + if (dev->of_node) { >> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); >> + if (!pdata) { >> + dev_err(dev, "Memory allocation failure\n"); >> + return -ENOMEM; >> + } >> + nop_xeiv_get_dt_pdata(dev, pdata); > > actually, I would prefer to not create pdata at all. I mean, ideally > pdata would be used to initialize fields in your own structure, so first > move clk_rate to your own private structure, copy pdata's clk_rate value > to that, then you don't need this hackery when using DT. As far as I can see, clk_rate is never used, but in the probe function. Why should it be saved into the private data structure at all? Marc
On 03/11/2013 09:40 AM, Roger Quadros wrote: > On 03/08/2013 05:45 PM, Marc Kleine-Budde wrote: >> On 03/08/2013 11:46 AM, Marc Kleine-Budde wrote: >>> On 02/04/2013 04:58 PM, Roger Quadros wrote: >>>> The PHY clock, clock rate, VCC regulator and RESET regulator >>>> can now be provided via device tree. >>>> >>>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>>> --- >>>> .../devicetree/bindings/usb/usb-nop-xceiv.txt | 34 ++++++++++++++++++++ >>>> drivers/usb/otg/nop-usb-xceiv.c | 31 ++++++++++++++++++ >>>> 2 files changed, 65 insertions(+), 0 deletions(-) >>>> create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >>>> new file mode 100644 >>>> index 0000000..d7e2726 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >>>> @@ -0,0 +1,34 @@ >>>> +USB NOP PHY >>>> + >>>> +Required properties: >>>> +- compatible: should be usb-nop-xceiv >>>> + >>>> +Optional properties: >>>> +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree >>>> + /bindings/clock/clock-bindings.txt >>>> + This property is required if clock-frequency is specified. >>>> + >>>> +- clock-names: Should be "main_clk" >>>> + >>>> +- clock-frequency: the clock frequency (in Hz) that the PHY clock must >>>> + be configured to. >>>> + >>>> +- vcc-supply: phandle to the regulator that provides RESET to the PHY. >>>> + >>>> +- reset-supply: phandle to the regulator that provides power to the PHY. >>>> + >>>> +Example: >>>> + >>>> + hsusb1_phy { >>>> + compatible = "usb-nop-xceiv"; >>>> + clock-frequency = <19200000>; >>> >>> Why do you hardcode the clock frequency here? You should use >>> clk_get_rate() to get the frequency from the clock tree. >> >> What about declaring a "fixed-clock" node in the device tree? Then it >> should be possible to keep the driver free of any omap specific code. >> > > The current implementation is not OMAP specific and is not limited to a > fixed frequency clock. The PHY driver is using standard clock APIs to set > the clock rate 'only' if the 'clock-frequency' property is present in the > device tree node. > > What is the benefit of declaring it as a "fixed-clock"? > FYI. The clock may not necessarily be a fixed frequency clock and someone > needs to program the rate. Okay, now I got it. Marc
On 03/11/2013 05:52 PM, Marc Kleine-Budde wrote: > On 02/05/2013 08:26 AM, Felipe Balbi wrote: >> Hi, >> >> On Mon, Feb 04, 2013 at 05:58:48PM +0200, Roger Quadros wrote: >>> The PHY clock, clock rate, VCC regulator and RESET regulator >>> can now be provided via device tree. >>> >>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>> --- >>> .../devicetree/bindings/usb/usb-nop-xceiv.txt | 34 ++++++++++++++++++++ >>> drivers/usb/otg/nop-usb-xceiv.c | 31 ++++++++++++++++++ >>> 2 files changed, 65 insertions(+), 0 deletions(-) >>> create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >>> >>> diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >>> new file mode 100644 >>> index 0000000..d7e2726 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt >>> @@ -0,0 +1,34 @@ >>> +USB NOP PHY >>> + >>> +Required properties: >>> +- compatible: should be usb-nop-xceiv >>> + >>> +Optional properties: >>> +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree >>> + /bindings/clock/clock-bindings.txt >>> + This property is required if clock-frequency is specified. >>> + >>> +- clock-names: Should be "main_clk" >>> + >>> +- clock-frequency: the clock frequency (in Hz) that the PHY clock must >>> + be configured to. >>> + >>> +- vcc-supply: phandle to the regulator that provides RESET to the PHY. >>> + >>> +- reset-supply: phandle to the regulator that provides power to the PHY. >>> + >>> +Example: >>> + >>> + hsusb1_phy { >>> + compatible = "usb-nop-xceiv"; >>> + clock-frequency = <19200000>; >>> + clocks = <&osc 0>; >>> + clock-names = "main_clk"; >>> + vcc-supply = <&hsusb1_vcc_regulator>; >>> + reset-supply = <&hsusb1_reset_regulator>; >>> + }; >>> + >>> +hsusb1_phy is a NOP USB PHY device that gets its clock from an oscillator >>> +and expects that clock to be configured to 19.2MHz by the NOP PHY driver. >>> +hsusb1_vcc_regulator provides power to the PHY and hsusb1_reset_regulator >>> +controls RESET. >>> diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c >>> index ac027a1..adbb7ab 100644 >>> --- a/drivers/usb/otg/nop-usb-xceiv.c >>> +++ b/drivers/usb/otg/nop-usb-xceiv.c >>> @@ -34,6 +34,7 @@ >>> #include <linux/slab.h> >>> #include <linux/clk.h> >>> #include <linux/regulator/consumer.h> >>> +#include <linux/of.h> >>> >>> struct nop_usb_xceiv { >>> struct usb_phy phy; >>> @@ -138,8 +139,19 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host) >>> return 0; >>> } >>> >>> +static void nop_xeiv_get_dt_pdata(struct device *dev, >> >> asking to remove, but xeiv != xceiv :-) >> >>> + struct nop_usb_xceiv_platform_data *pdata) >>> +{ >>> + struct device_node *node = dev->of_node; >>> + u32 clk_rate; >>> + >>> + if (!of_property_read_u32(node, "clock-frequency", &clk_rate)) >>> + pdata->clk_rate = clk_rate; >>> +} >>> + >>> static int nop_usb_xceiv_probe(struct platform_device *pdev) >>> { >>> + struct device *dev = &pdev->dev; >>> struct nop_usb_xceiv_platform_data *pdata = pdev->dev.platform_data; >>> struct nop_usb_xceiv *nop; >>> enum usb_phy_type type = USB_PHY_TYPE_USB2; >>> @@ -153,6 +165,17 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev) >>> if (!nop->phy.otg) >>> return -ENOMEM; >>> >>> + if (dev->of_node) { >>> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); >>> + if (!pdata) { >>> + dev_err(dev, "Memory allocation failure\n"); >>> + return -ENOMEM; >>> + } >>> + nop_xeiv_get_dt_pdata(dev, pdata); >> >> actually, I would prefer to not create pdata at all. I mean, ideally >> pdata would be used to initialize fields in your own structure, so first >> move clk_rate to your own private structure, copy pdata's clk_rate value >> to that, then you don't need this hackery when using DT. > > As far as I can see, clk_rate is never used, but in the probe function. > Why should it be saved into the private data structure at all? > Yes you are right. I'll fix it up. Thanks. cheers, -roger
diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt new file mode 100644 index 0000000..d7e2726 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt @@ -0,0 +1,34 @@ +USB NOP PHY + +Required properties: +- compatible: should be usb-nop-xceiv + +Optional properties: +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree + /bindings/clock/clock-bindings.txt + This property is required if clock-frequency is specified. + +- clock-names: Should be "main_clk" + +- clock-frequency: the clock frequency (in Hz) that the PHY clock must + be configured to. + +- vcc-supply: phandle to the regulator that provides RESET to the PHY. + +- reset-supply: phandle to the regulator that provides power to the PHY. + +Example: + + hsusb1_phy { + compatible = "usb-nop-xceiv"; + clock-frequency = <19200000>; + clocks = <&osc 0>; + clock-names = "main_clk"; + vcc-supply = <&hsusb1_vcc_regulator>; + reset-supply = <&hsusb1_reset_regulator>; + }; + +hsusb1_phy is a NOP USB PHY device that gets its clock from an oscillator +and expects that clock to be configured to 19.2MHz by the NOP PHY driver. +hsusb1_vcc_regulator provides power to the PHY and hsusb1_reset_regulator +controls RESET. diff --git a/drivers/usb/otg/nop-usb-xceiv.c b/drivers/usb/otg/nop-usb-xceiv.c index ac027a1..adbb7ab 100644 --- a/drivers/usb/otg/nop-usb-xceiv.c +++ b/drivers/usb/otg/nop-usb-xceiv.c @@ -34,6 +34,7 @@ #include <linux/slab.h> #include <linux/clk.h> #include <linux/regulator/consumer.h> +#include <linux/of.h> struct nop_usb_xceiv { struct usb_phy phy; @@ -138,8 +139,19 @@ static int nop_set_host(struct usb_otg *otg, struct usb_bus *host) return 0; } +static void nop_xeiv_get_dt_pdata(struct device *dev, + struct nop_usb_xceiv_platform_data *pdata) +{ + struct device_node *node = dev->of_node; + u32 clk_rate; + + if (!of_property_read_u32(node, "clock-frequency", &clk_rate)) + pdata->clk_rate = clk_rate; +} + static int nop_usb_xceiv_probe(struct platform_device *pdev) { + struct device *dev = &pdev->dev; struct nop_usb_xceiv_platform_data *pdata = pdev->dev.platform_data; struct nop_usb_xceiv *nop; enum usb_phy_type type = USB_PHY_TYPE_USB2; @@ -153,6 +165,17 @@ static int nop_usb_xceiv_probe(struct platform_device *pdev) if (!nop->phy.otg) return -ENOMEM; + if (dev->of_node) { + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) { + dev_err(dev, "Memory allocation failure\n"); + return -ENOMEM; + } + nop_xeiv_get_dt_pdata(dev, pdata); + } else { + pdata = dev->platform_data; + } + if (pdata) type = pdata->type; @@ -236,12 +259,20 @@ static int nop_usb_xceiv_remove(struct platform_device *pdev) return 0; } +static const struct of_device_id nop_xceiv_dt_ids[] = { + { .compatible = "usb-nop-xceiv" }, + { } +}; + +MODULE_DEVICE_TABLE(of, nop_xceiv_dt_ids); + static struct platform_driver nop_usb_xceiv_driver = { .probe = nop_usb_xceiv_probe, .remove = nop_usb_xceiv_remove, .driver = { .name = "nop_usb_xceiv", .owner = THIS_MODULE, + .of_match_table = of_match_ptr(nop_xceiv_dt_ids), }, };
The PHY clock, clock rate, VCC regulator and RESET regulator can now be provided via device tree. Signed-off-by: Roger Quadros <rogerq@ti.com> --- .../devicetree/bindings/usb/usb-nop-xceiv.txt | 34 ++++++++++++++++++++ drivers/usb/otg/nop-usb-xceiv.c | 31 ++++++++++++++++++ 2 files changed, 65 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt