Re: [PATCH] usb: phy: samsung: Add support for USB 3.0 phy for exynos5250

2012-12-18 Thread Felipe Balbi
On Tue, Dec 18, 2012 at 08:40:26PM +0530, Vivek Gautam wrote:
> Adding support for USB3.0 phy for dwc3 controller on
> exynos5250 SOC.
> 
> Signed-off-by: Vivek Gautam 
> ---
>  drivers/usb/phy/samsung-usbphy.c |  339 
> +-

let's make the phy names standard from now on and call this
phy-samsung.c :-)

>  1 files changed, 337 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/phy/samsung-usbphy.c 
> b/drivers/usb/phy/samsung-usbphy.c
> index 621348a..246eb28 100644
> --- a/drivers/usb/phy/samsung-usbphy.c
> +++ b/drivers/usb/phy/samsung-usbphy.c
> @@ -154,6 +154,86 @@
>  
>  #define EXYNOS5_PHY_OTG_TUNE (0x40)
>  
> +/* EXYNOS5: USB 3.0 DRD */
> +#define EXYNOS5_DRD_LINKSYSTEM   (0x04)
> +
> +#define LINKSYSTEM_FLADJ_MASK(0x3f << 1)
> +#define LINKSYSTEM_FLADJ(_x) ((_x) << 1)
> +#define LINKSYSTEM_XHCI_VERSION_CONTROL  (0x1 << 27)
> +
> +#define EXYNOS5_DRD_PHYUTMI  (0x08)
> +
> +#define PHYUTMI_OTGDISABLE   (0x1 << 6)
> +#define PHYUTMI_FORCESUSPEND (0x1 << 1)
> +#define PHYUTMI_FORCESLEEP   (0x1 << 0)
> +
> +#define EXYNOS5_DRD_PHYPIPE  (0x0c)
> +
> +#define EXYNOS5_DRD_PHYCLKRST(0x10)
> +
> +#define PHYCLKRST_SSC_REFCLKSEL_MASK (0xff << 23)
> +#define PHYCLKRST_SSC_REFCLKSEL(_x)  ((_x) << 23)
> +
> +#define PHYCLKRST_SSC_RANGE_MASK (0x03 << 21)
> +#define PHYCLKRST_SSC_RANGE(_x)  ((_x) << 21)
> +
> +#define PHYCLKRST_SSC_EN (0x1 << 20)
> +#define PHYCLKRST_REF_SSP_EN (0x1 << 19)
> +#define PHYCLKRST_REF_CLKDIV2(0x1 << 18)
> +
> +#define PHYCLKRST_MPLL_MULTIPLIER_MASK   (0x7f << 11)
> +#define PHYCLKRST_MPLL_MULTIPLIER_100MHZ_REF (0x19 << 11)
> +#define PHYCLKRST_MPLL_MULTIPLIER_50M_REF(0x02 << 11)
> +#define PHYCLKRST_MPLL_MULTIPLIER_24MHZ_REF  (0x68 << 11)
> +#define PHYCLKRST_MPLL_MULTIPLIER_20MHZ_REF  (0x7d << 11)
> +#define PHYCLKRST_MPLL_MULTIPLIER_19200KHZ_REF   (0x02 << 11)
> +
> +#define PHYCLKRST_FSEL_MASK  (0x3f << 5)
> +#define PHYCLKRST_FSEL(_x)   ((_x) << 5)
> +#define PHYCLKRST_FSEL_PAD_100MHZ(0x27 << 5)
> +#define PHYCLKRST_FSEL_PAD_24MHZ (0x2a << 5)
> +#define PHYCLKRST_FSEL_PAD_20MHZ (0x31 << 5)
> +#define PHYCLKRST_FSEL_PAD_19_2MHZ   (0x38 << 5)
> +
> +#define PHYCLKRST_RETENABLEN (0x1 << 4)
> +
> +#define PHYCLKRST_REFCLKSEL_MASK (0x03 << 2)
> +#define PHYCLKRST_REFCLKSEL_PAD_REFCLK   (0x2 << 2)
> +#define PHYCLKRST_REFCLKSEL_EXT_REFCLK   (0x3 << 2)
> +
> +#define PHYCLKRST_PORTRESET  (0x1 << 1)
> +#define PHYCLKRST_COMMONONN  (0x1 << 0)
> +
> +#define EXYNOS5_DRD_PHYREG0  (0x14)
> +#define EXYNOS5_DRD_PHYREG1  (0x18)
> +
> +#define EXYNOS5_DRD_PHYPARAM0(0x1c)
> +
> +#define PHYPARAM0_REF_USE_PAD(0x1 << 31)
> +#define PHYPARAM0_REF_LOSLEVEL_MASK  (0x1f << 26)
> +#define PHYPARAM0_REF_LOSLEVEL   (0x9 << 26)
> +
> +#define EXYNOS5_DRD_PHYPARAM1(0x20)
> +
> +#define PHYPARAM1_PCS_TXDEEMPH_MASK  (0x1f << 0)
> +#define PHYPARAM1_PCS_TXDEEMPH   (0x1c)
> +
> +#define EXYNOS5_DRD_PHYTERM  (0x24)
> +
> +#define EXYNOS5_DRD_PHYTEST  (0x28)
> +
> +#define PHYTEST_POWERDOWN_SSP(0x1 << 3)
> +#define PHYTEST_POWERDOWN_HSP(0x1 << 2)
> +
> +#define EXYNOS5_DRD_PHYADP   (0x2c)
> +
> +#define EXYNOS5_DRD_PHYBATCHG(0x30)
> +
> +#define PHYBATCHG_UTMI_CLKSEL(0x1 << 2)
> +
> +#define EXYNOS5_DRD_PHYRESUME(0x34)
> +#define EXYNOS5_DRD_LINKPORT (0x44)
> +
>  #ifndef MHZ
>  #define MHZ (1000*1000)
>  #endif
> @@ -164,6 +244,15 @@ enum samsung_cpu_type {
>   TYPE_EXYNOS5250,
>  };
>  
> +/* structure usb3 - usb3.0 phy trasceiver state
> + * @phy: transceiver structure for USB 3.0
> + * @regs_phy: usb 3.0 phy register memory base
> + */
> +struct usb3 {
> + struct usb_phy  phy;
> + void __iomem*regs_phy;
> +};
> +
>  /*
>   * struct samsung_usbphy - transceiver driver state
>   * @phy: transceiver structure
> @@ -192,11 +281,15 @@ struct samsung_usbphy {
>   u32 en_mask;
>   int ref_clk_freq;
>   int cpu_type;
> + struct usb3 usb3phy;
> + int has_usb3;
>   enum samsung_usb_phy_type phy_type;
>   atomic_thost_usage;
>  };
>  
>  #define phy_to_sphy(x)   container_of((x), struct 
> samsung_usbphy, phy)
> +#define phy_to_usb3phy(x)container_of

Re: [PATCH] usb: phy: samsung: Add support for USB 3.0 phy for exynos5250

2012-12-18 Thread Vivek Gautam
CC: Doug Anderson


On Tue, Dec 18, 2012 at 8:43 PM, Felipe Balbi  wrote:
> On Tue, Dec 18, 2012 at 08:40:26PM +0530, Vivek Gautam wrote:
>> Adding support for USB3.0 phy for dwc3 controller on
>> exynos5250 SOC.
>>
>> Signed-off-by: Vivek Gautam 
>> ---
>>  drivers/usb/phy/samsung-usbphy.c |  339 
>> +-
>
> let's make the phy names standard from now on and call this
> phy-samsung.c :-)
>
>>  1 files changed, 337 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/phy/samsung-usbphy.c 
>> b/drivers/usb/phy/samsung-usbphy.c
>> index 621348a..246eb28 100644
>> --- a/drivers/usb/phy/samsung-usbphy.c
>> +++ b/drivers/usb/phy/samsung-usbphy.c
>> @@ -154,6 +154,86 @@
>>
>>  #define EXYNOS5_PHY_OTG_TUNE (0x40)
>>
>> +/* EXYNOS5: USB 3.0 DRD */
>> +#define EXYNOS5_DRD_LINKSYSTEM   (0x04)
>> +
>> +#define LINKSYSTEM_FLADJ_MASK(0x3f << 1)
>> +#define LINKSYSTEM_FLADJ(_x) ((_x) << 1)
>> +#define LINKSYSTEM_XHCI_VERSION_CONTROL  (0x1 << 27)
>> +
>> +#define EXYNOS5_DRD_PHYUTMI  (0x08)
>> +
>> +#define PHYUTMI_OTGDISABLE   (0x1 << 6)
>> +#define PHYUTMI_FORCESUSPEND (0x1 << 1)
>> +#define PHYUTMI_FORCESLEEP   (0x1 << 0)
>> +
>> +#define EXYNOS5_DRD_PHYPIPE  (0x0c)
>> +
>> +#define EXYNOS5_DRD_PHYCLKRST(0x10)
>> +
>> +#define PHYCLKRST_SSC_REFCLKSEL_MASK (0xff << 23)
>> +#define PHYCLKRST_SSC_REFCLKSEL(_x)  ((_x) << 23)
>> +
>> +#define PHYCLKRST_SSC_RANGE_MASK (0x03 << 21)
>> +#define PHYCLKRST_SSC_RANGE(_x)  ((_x) << 21)
>> +
>> +#define PHYCLKRST_SSC_EN (0x1 << 20)
>> +#define PHYCLKRST_REF_SSP_EN (0x1 << 19)
>> +#define PHYCLKRST_REF_CLKDIV2(0x1 << 18)
>> +
>> +#define PHYCLKRST_MPLL_MULTIPLIER_MASK   (0x7f << 11)
>> +#define PHYCLKRST_MPLL_MULTIPLIER_100MHZ_REF (0x19 << 11)
>> +#define PHYCLKRST_MPLL_MULTIPLIER_50M_REF(0x02 << 11)
>> +#define PHYCLKRST_MPLL_MULTIPLIER_24MHZ_REF  (0x68 << 11)
>> +#define PHYCLKRST_MPLL_MULTIPLIER_20MHZ_REF  (0x7d << 11)
>> +#define PHYCLKRST_MPLL_MULTIPLIER_19200KHZ_REF   (0x02 << 11)
>> +
>> +#define PHYCLKRST_FSEL_MASK  (0x3f << 5)
>> +#define PHYCLKRST_FSEL(_x)   ((_x) << 5)
>> +#define PHYCLKRST_FSEL_PAD_100MHZ(0x27 << 5)
>> +#define PHYCLKRST_FSEL_PAD_24MHZ (0x2a << 5)
>> +#define PHYCLKRST_FSEL_PAD_20MHZ (0x31 << 5)
>> +#define PHYCLKRST_FSEL_PAD_19_2MHZ   (0x38 << 5)
>> +
>> +#define PHYCLKRST_RETENABLEN (0x1 << 4)
>> +
>> +#define PHYCLKRST_REFCLKSEL_MASK (0x03 << 2)
>> +#define PHYCLKRST_REFCLKSEL_PAD_REFCLK   (0x2 << 2)
>> +#define PHYCLKRST_REFCLKSEL_EXT_REFCLK   (0x3 << 2)
>> +
>> +#define PHYCLKRST_PORTRESET  (0x1 << 1)
>> +#define PHYCLKRST_COMMONONN  (0x1 << 0)
>> +
>> +#define EXYNOS5_DRD_PHYREG0  (0x14)
>> +#define EXYNOS5_DRD_PHYREG1  (0x18)
>> +
>> +#define EXYNOS5_DRD_PHYPARAM0(0x1c)
>> +
>> +#define PHYPARAM0_REF_USE_PAD(0x1 << 31)
>> +#define PHYPARAM0_REF_LOSLEVEL_MASK  (0x1f << 26)
>> +#define PHYPARAM0_REF_LOSLEVEL   (0x9 << 26)
>> +
>> +#define EXYNOS5_DRD_PHYPARAM1(0x20)
>> +
>> +#define PHYPARAM1_PCS_TXDEEMPH_MASK  (0x1f << 0)
>> +#define PHYPARAM1_PCS_TXDEEMPH   (0x1c)
>> +
>> +#define EXYNOS5_DRD_PHYTERM  (0x24)
>> +
>> +#define EXYNOS5_DRD_PHYTEST  (0x28)
>> +
>> +#define PHYTEST_POWERDOWN_SSP(0x1 << 3)
>> +#define PHYTEST_POWERDOWN_HSP(0x1 << 2)
>> +
>> +#define EXYNOS5_DRD_PHYADP   (0x2c)
>> +
>> +#define EXYNOS5_DRD_PHYBATCHG(0x30)
>> +
>> +#define PHYBATCHG_UTMI_CLKSEL(0x1 << 2)
>> +
>> +#define EXYNOS5_DRD_PHYRESUME(0x34)
>> +#define EXYNOS5_DRD_LINKPORT (0x44)
>> +
>>  #ifndef MHZ
>>  #define MHZ (1000*1000)
>>  #endif
>> @@ -164,6 +244,15 @@ enum samsung_cpu_type {
>>   TYPE_EXYNOS5250,
>>  };
>>
>> +/* structure usb3 - usb3.0 phy trasceiver state
>> + * @phy: transceiver structure for USB 3.0
>> + * @regs_phy: usb 3.0 phy register memory base
>> + */
>> +struct usb3 {
>> + struct usb_phy  phy;
>> + void __iomem*regs_phy;
>> +};
>> +
>>  /*
>>   * struct samsung_usbphy - transceiver driver state
>>   * @phy: transceiver structure
>> @@ -192,11 +281,15 @@ struct samsung_usbphy {
>>   u32 en_mask;
>>   int ref_clk_freq;
>>   int cpu_type;
>> + struct usb3 usb3phy;
>> + int has_usb3;
>>   enum samsung_usb_

Re: [PATCH] usb: phy: samsung: Add support for USB 3.0 phy for exynos5250

2012-12-18 Thread Vivek Gautam
Hi Felipe,


On Wed, Dec 19, 2012 at 11:14 AM, Vivek Gautam
 wrote:
> CC: Doug Anderson
>
>
> On Tue, Dec 18, 2012 at 8:43 PM, Felipe Balbi  wrote:
>> On Tue, Dec 18, 2012 at 08:40:26PM +0530, Vivek Gautam wrote:
>>> Adding support for USB3.0 phy for dwc3 controller on
>>> exynos5250 SOC.
>>>
>>> Signed-off-by: Vivek Gautam 
>>> ---
>>>  drivers/usb/phy/samsung-usbphy.c |  339 
>>> +-
>>
>> let's make the phy names standard from now on and call this
>> phy-samsung.c :-)
>>
>>>  1 files changed, 337 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/usb/phy/samsung-usbphy.c 
>>> b/drivers/usb/phy/samsung-usbphy.c
>>> index 621348a..246eb28 100644
>>> --- a/drivers/usb/phy/samsung-usbphy.c
>>> +++ b/drivers/usb/phy/samsung-usbphy.c
>>> @@ -154,6 +154,86 @@
>>>
>>>  #define EXYNOS5_PHY_OTG_TUNE (0x40)
>>>
>>> +/* EXYNOS5: USB 3.0 DRD */
>>> +#define EXYNOS5_DRD_LINKSYSTEM   (0x04)
>>> +
>>> +#define LINKSYSTEM_FLADJ_MASK(0x3f << 1)
>>> +#define LINKSYSTEM_FLADJ(_x) ((_x) << 1)
>>> +#define LINKSYSTEM_XHCI_VERSION_CONTROL  (0x1 << 27)
>>> +
>>> +#define EXYNOS5_DRD_PHYUTMI  (0x08)
>>> +
>>> +#define PHYUTMI_OTGDISABLE   (0x1 << 6)
>>> +#define PHYUTMI_FORCESUSPEND (0x1 << 1)
>>> +#define PHYUTMI_FORCESLEEP   (0x1 << 0)
>>> +
>>> +#define EXYNOS5_DRD_PHYPIPE  (0x0c)
>>> +
>>> +#define EXYNOS5_DRD_PHYCLKRST(0x10)
>>> +
>>> +#define PHYCLKRST_SSC_REFCLKSEL_MASK (0xff << 23)
>>> +#define PHYCLKRST_SSC_REFCLKSEL(_x)  ((_x) << 23)
>>> +
>>> +#define PHYCLKRST_SSC_RANGE_MASK (0x03 << 21)
>>> +#define PHYCLKRST_SSC_RANGE(_x)  ((_x) << 21)
>>> +
>>> +#define PHYCLKRST_SSC_EN (0x1 << 20)
>>> +#define PHYCLKRST_REF_SSP_EN (0x1 << 19)
>>> +#define PHYCLKRST_REF_CLKDIV2(0x1 << 18)
>>> +
>>> +#define PHYCLKRST_MPLL_MULTIPLIER_MASK   (0x7f << 11)
>>> +#define PHYCLKRST_MPLL_MULTIPLIER_100MHZ_REF (0x19 << 11)
>>> +#define PHYCLKRST_MPLL_MULTIPLIER_50M_REF(0x02 << 11)
>>> +#define PHYCLKRST_MPLL_MULTIPLIER_24MHZ_REF  (0x68 << 11)
>>> +#define PHYCLKRST_MPLL_MULTIPLIER_20MHZ_REF  (0x7d << 11)
>>> +#define PHYCLKRST_MPLL_MULTIPLIER_19200KHZ_REF   (0x02 << 11)
>>> +
>>> +#define PHYCLKRST_FSEL_MASK  (0x3f << 5)
>>> +#define PHYCLKRST_FSEL(_x)   ((_x) << 5)
>>> +#define PHYCLKRST_FSEL_PAD_100MHZ(0x27 << 5)
>>> +#define PHYCLKRST_FSEL_PAD_24MHZ (0x2a << 5)
>>> +#define PHYCLKRST_FSEL_PAD_20MHZ (0x31 << 5)
>>> +#define PHYCLKRST_FSEL_PAD_19_2MHZ   (0x38 << 5)
>>> +
>>> +#define PHYCLKRST_RETENABLEN (0x1 << 4)
>>> +
>>> +#define PHYCLKRST_REFCLKSEL_MASK (0x03 << 2)
>>> +#define PHYCLKRST_REFCLKSEL_PAD_REFCLK   (0x2 << 2)
>>> +#define PHYCLKRST_REFCLKSEL_EXT_REFCLK   (0x3 << 2)
>>> +
>>> +#define PHYCLKRST_PORTRESET  (0x1 << 1)
>>> +#define PHYCLKRST_COMMONONN  (0x1 << 0)
>>> +
>>> +#define EXYNOS5_DRD_PHYREG0  (0x14)
>>> +#define EXYNOS5_DRD_PHYREG1  (0x18)
>>> +
>>> +#define EXYNOS5_DRD_PHYPARAM0(0x1c)
>>> +
>>> +#define PHYPARAM0_REF_USE_PAD(0x1 << 31)
>>> +#define PHYPARAM0_REF_LOSLEVEL_MASK  (0x1f << 26)
>>> +#define PHYPARAM0_REF_LOSLEVEL   (0x9 << 26)
>>> +
>>> +#define EXYNOS5_DRD_PHYPARAM1(0x20)
>>> +
>>> +#define PHYPARAM1_PCS_TXDEEMPH_MASK  (0x1f << 0)
>>> +#define PHYPARAM1_PCS_TXDEEMPH   (0x1c)
>>> +
>>> +#define EXYNOS5_DRD_PHYTERM  (0x24)
>>> +
>>> +#define EXYNOS5_DRD_PHYTEST  (0x28)
>>> +
>>> +#define PHYTEST_POWERDOWN_SSP(0x1 << 3)
>>> +#define PHYTEST_POWERDOWN_HSP(0x1 << 2)
>>> +
>>> +#define EXYNOS5_DRD_PHYADP   (0x2c)
>>> +
>>> +#define EXYNOS5_DRD_PHYBATCHG(0x30)
>>> +
>>> +#define PHYBATCHG_UTMI_CLKSEL(0x1 << 2)
>>> +
>>> +#define EXYNOS5_DRD_PHYRESUME(0x34)
>>> +#define EXYNOS5_DRD_LINKPORT (0x44)
>>> +
>>>  #ifndef MHZ
>>>  #define MHZ (1000*1000)
>>>  #endif
>>> @@ -164,6 +244,15 @@ enum samsung_cpu_type {
>>>   TYPE_EXYNOS5250,
>>>  };
>>>
>>> +/* structure usb3 - usb3.0 phy trasceiver state
>>> + * @phy: transceiver structure for USB 3.0
>>> + * @regs_phy: usb 3.0 phy register memory base
>>> + */
>>> +struct usb3 {
>>> + struct usb_phy  phy;
>>> + void __iomem*regs_phy;
>>> +};
>>> +
>>>  /*
>>>   * struct samsung_usbphy - transceiver driver state
>>>   * @phy: transceiver structure
>>> @@ -192,11 +281,15 @@ struct samsung_usbphy {
>>>   

Re: [PATCH] usb: phy: samsung: Add support for USB 3.0 phy for exynos5250

2012-12-19 Thread Felipe Balbi
Hi,

On Wed, Dec 19, 2012 at 11:52:01AM +0530, Vivek Gautam wrote:
> >>> @@ -736,7 +1035,41 @@ static int __devinit samsung_usbphy_probe(struct 
> >>> platform_device *pdev)
> >>>
> >>>   sphy->clk = clk;
> >>>
> >>> - return usb_add_phy(&sphy->phy, USB_PHY_TYPE_USB2);
> >>> + sphy->has_usb3 = (sphy->cpu_type == TYPE_EXYNOS5250);
> >>> +
> >>> + if (sphy->has_usb3) {
> >>> + struct resource *usb3phy_mem;
> >>> + void __iomem*usb3phy_base;
> >>> +
> >>> + usb3phy_mem = platform_get_resource(pdev, IORESOURCE_MEM, 
> >>> 1);
> >>> + if (!usb3phy_mem) {
> >>> + dev_err(dev, "%s: missing mem resource\n", 
> >>> __func__);
> >>> + return -ENODEV;
> >>> + }
> >>> +
> >>> + usb3phy_base = devm_request_and_ioremap(dev, usb3phy_mem);
> >>> + if (!usb3phy_base) {
> >>> + dev_err(dev, "%s: register mapping failed\n", 
> >>> __func__);
> >>> + return -ENXIO;
> >>> + }
> >>> +
> >>> + sphy->usb3phy.regs_phy  = usb3phy_base;
> >>> + sphy->usb3phy.phy.dev   = sphy->dev;
> >>> + sphy->usb3phy.phy.label = "samsung-usb3phy";
> >>> + sphy->usb3phy.phy.init  = samsung_usb3phy_init;
> >>> + sphy->usb3phy.phy.shutdown  = samsung_usb3phy_shutdown;
> >>> + }
> >>> +
> >>> + ret = usb_add_phy(&sphy->phy, USB_PHY_TYPE_USB2);
> >>> + if (ret)
> >>> + return ret;
> >>
> >> is this realy how your HW behaves ? USB2 and USB3 phys are a single HW
> >> entity ? I kinda doubt that :-s
> >>
> 
> They are separate HW in fact.
> So, do you expect to see a separate driver interface for USB 3.0 type phy ?

yes. Just as we did on OMAP. One driver for the USB2 part and one driver
for USB3 part (which are actually two, but you can only talk to them as
one) :-)

> That will be quite similar architecturally to current samsung-usbphy
> driver for USB 2.0 type phy,
> and may require some code duplication too.

If it duplicates code, then perhaps it's best to keep it as is but I'm
actually surprised you guys have similar programming model on both
parts. I mean, the differences at HW behavior are huge: on one side you
use ULPI/UTMI+ on the other PIPE3, on one side you have 480Mbps
half-duplex signalling, on the other you have 5Gbps dual simplex
signalling, the differences go on and on.

Also, what you say about duplicating, it seems to me that it will
duplicate only the boylerplate part (allocating memory, having a
platform_driver, and so on), because you _do_ have completely separate
functions to handle usb3 part.

One more comment below:

> +static u32 exynos5_usb3phy_set_clock(struct samsung_usbphy *sphy)
> +{
> + u32 reg;
> + u32 refclk;
> +
> + refclk = sphy->ref_clk_freq;
> +
> + reg = PHYCLKRST_REFCLKSEL_EXT_REFCLK |
> + PHYCLKRST_FSEL(refclk);
> +
> + switch (refclk) {
> + case HOST_CTRL0_FSEL_CLKSEL_50M:
> + reg |= (PHYCLKRST_MPLL_MULTIPLIER_50M_REF |
> + PHYCLKRST_SSC_REFCLKSEL(0x00));
> + break;
> + case HOST_CTRL0_FSEL_CLKSEL_20M:
> + reg |= (PHYCLKRST_MPLL_MULTIPLIER_20MHZ_REF |
> + PHYCLKRST_SSC_REFCLKSEL(0x00));
> + break;
> + case HOST_CTRL0_FSEL_CLKSEL_19200K:
> + reg |= (PHYCLKRST_MPLL_MULTIPLIER_19200KHZ_REF |
> + PHYCLKRST_SSC_REFCLKSEL(0x88));
> + break;
> + case HOST_CTRL0_FSEL_CLKSEL_24M:
> + default:
> + reg |= (PHYCLKRST_MPLL_MULTIPLIER_24MHZ_REF |
> + PHYCLKRST_SSC_REFCLKSEL(0x88));
> + break;
> + }
> +
> + return reg;
> +}

looks like this should be done by commong clock framework by clock
reparenting perhaps ?

-- 
balbi


signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH] usb: phy: samsung: Add support for USB 3.0 phy for exynos5250

2012-12-21 Thread Vivek Gautam
Hi Felipe,


On Wed, Dec 19, 2012 at 1:25 PM, Felipe Balbi  wrote:
> Hi,
>
> On Wed, Dec 19, 2012 at 11:52:01AM +0530, Vivek Gautam wrote:
>> >>> @@ -736,7 +1035,41 @@ static int __devinit samsung_usbphy_probe(struct 
>> >>> platform_device *pdev)
>> >>>
>> >>>   sphy->clk = clk;
>> >>>
>> >>> - return usb_add_phy(&sphy->phy, USB_PHY_TYPE_USB2);
>> >>> + sphy->has_usb3 = (sphy->cpu_type == TYPE_EXYNOS5250);
>> >>> +
>> >>> + if (sphy->has_usb3) {
>> >>> + struct resource *usb3phy_mem;
>> >>> + void __iomem*usb3phy_base;
>> >>> +
>> >>> + usb3phy_mem = platform_get_resource(pdev, IORESOURCE_MEM, 
>> >>> 1);
>> >>> + if (!usb3phy_mem) {
>> >>> + dev_err(dev, "%s: missing mem resource\n", 
>> >>> __func__);
>> >>> + return -ENODEV;
>> >>> + }
>> >>> +
>> >>> + usb3phy_base = devm_request_and_ioremap(dev, usb3phy_mem);
>> >>> + if (!usb3phy_base) {
>> >>> + dev_err(dev, "%s: register mapping failed\n", 
>> >>> __func__);
>> >>> + return -ENXIO;
>> >>> + }
>> >>> +
>> >>> + sphy->usb3phy.regs_phy  = usb3phy_base;
>> >>> + sphy->usb3phy.phy.dev   = sphy->dev;
>> >>> + sphy->usb3phy.phy.label = "samsung-usb3phy";
>> >>> + sphy->usb3phy.phy.init  = samsung_usb3phy_init;
>> >>> + sphy->usb3phy.phy.shutdown  = samsung_usb3phy_shutdown;
>> >>> + }
>> >>> +
>> >>> + ret = usb_add_phy(&sphy->phy, USB_PHY_TYPE_USB2);
>> >>> + if (ret)
>> >>> + return ret;
>> >>
>> >> is this realy how your HW behaves ? USB2 and USB3 phys are a single HW
>> >> entity ? I kinda doubt that :-s
>> >>
>>
>> They are separate HW in fact.
>> So, do you expect to see a separate driver interface for USB 3.0 type phy ?
>
> yes. Just as we did on OMAP. One driver for the USB2 part and one driver
> for USB3 part (which are actually two, but you can only talk to them as
> one) :-)
>
Sure as you suggest, we will pull out the USB 3.0 code from here and put
a new driver in place for the same.

>> That will be quite similar architecturally to current samsung-usbphy
>> driver for USB 2.0 type phy,
>> and may require some code duplication too.
>
> If it duplicates code, then perhaps it's best to keep it as is but I'm
> actually surprised you guys have similar programming model on both
> parts. I mean, the differences at HW behavior are huge: on one side you
> use ULPI/UTMI+ on the other PIPE3, on one side you have 480Mbps
> half-duplex signalling, on the other you have 5Gbps dual simplex
> signalling, the differences go on and on.
>
The programming model for USB phy is just about setting up the phy registers
for example to provide proper clocks to PHY controller, setting up setting up
the UTMI block etc, under a sequence.

> Also, what you say about duplicating, it seems to me that it will
> duplicate only the boilerplate part (allocating memory, having a
> platform_driver, and so on), because you _do_ have completely separate
> functions to handle usb3 part.
>
Yes, the duplication was in fact just the boilerplate part ;-)
apart from having separate functions for USB 3.0 type phy.

> One more comment below:
>
>> +static u32 exynos5_usb3phy_set_clock(struct samsung_usbphy *sphy)
>> +{
>> + u32 reg;
>> + u32 refclk;
>> +
>> + refclk = sphy->ref_clk_freq;
>> +
>> + reg = PHYCLKRST_REFCLKSEL_EXT_REFCLK |
>> + PHYCLKRST_FSEL(refclk);
>> +
>> + switch (refclk) {
>> + case HOST_CTRL0_FSEL_CLKSEL_50M:
>> + reg |= (PHYCLKRST_MPLL_MULTIPLIER_50M_REF |
>> + PHYCLKRST_SSC_REFCLKSEL(0x00));
>> + break;
>> + case HOST_CTRL0_FSEL_CLKSEL_20M:
>> + reg |= (PHYCLKRST_MPLL_MULTIPLIER_20MHZ_REF |
>> + PHYCLKRST_SSC_REFCLKSEL(0x00));
>> + break;
>> + case HOST_CTRL0_FSEL_CLKSEL_19200K:
>> + reg |= (PHYCLKRST_MPLL_MULTIPLIER_19200KHZ_REF |
>> + PHYCLKRST_SSC_REFCLKSEL(0x88));
>> + break;
>> + case HOST_CTRL0_FSEL_CLKSEL_24M:
>> + default:
>> + reg |= (PHYCLKRST_MPLL_MULTIPLIER_24MHZ_REF |
>> + PHYCLKRST_SSC_REFCLKSEL(0x88));
>> + break;
>> + }
>> +
>> + return reg;
>> +}
>
> looks like this should be done by commong clock framework by clock
> reparenting perhaps ?
>
Need to check on this one.

> --
> balbi



-- 
Thanks & Regards
Vivek
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss