Hi,
On Thursday 22 May 2014 03:53 AM, Sergei Shtylyov wrote:
> Hello.
>
> On 04/23/2014 02:02 PM, Kishon Vijay Abraham I wrote:
>
>>>>> This PHY, though formally being a part of Renesas USBHS controller, contains
>>>>> the
>>>>> UGCTRL2 register that controls multiplexing of the USB ports (Renesas calls
>>>>> them
>>>>> channels) to the different USB controllers: channel 0 can be connected to
>>>>> either
>>>>> PCI EHCI/OHCI or USBHS controllers, channel 2 can be connected to PCI
>>>>> EHCI/OHCI
>>>>> or xHCI controllers.
>
>>>>> This is a new driver for this USB PHY currently already supported under
>>>>> drivers/
>>>>> usb/phy/. The reason for writing the new driver was the requirement that the
>>>>> multiplexing of USB channels to the controller be dynamic, depending on what
>>>>> USB drivers are loaded, rather than static as provided by the old driver.
>>>>> The infrastructure provided by drivers/phy/phy-core.c seems to fit that
>>>>> purpose
>>>>> ideally. The new driver only supports device tree probing for now.
>
>>>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>
>
>>> [...]
>
>>>>> Index: linux-phy/drivers/phy/phy-rcar-gen2.c
>>>>> ===================================================================
>>>>> --- /dev/null
>>>>> +++ linux-phy/drivers/phy/phy-rcar-gen2.c
>>>>> @@ -0,0 +1,283 @@
>
>>> [...]
>
>>>>> +static int rcar_gen2_phy_probe(struct platform_device *pdev)
>>>>> +{
>
>>> [...]
>
>>>>> + drv->phys[0][0].select_mask = USBHS_UGCTRL2_USB0SEL;
>>>>> + drv->phys[0][0].select_value = USBHS_UGCTRL2_USB0SEL_PCI;
>>>>> + drv->phys[0][1].select_mask = USBHS_UGCTRL2_USB0SEL;
>>>>> + drv->phys[0][1].select_value = USBHS_UGCTRL2_USB0SEL_HS_USB;
>>>>> + drv->phys[2][0].select_mask = USBHS_UGCTRL2_USB2SEL;
>>>>> + drv->phys[2][0].select_value = USBHS_UGCTRL2_USB2SEL_PCI;
>>>>> + drv->phys[2][1].select_mask = USBHS_UGCTRL2_USB2SEL;
>>>>> + drv->phys[2][1].select_value = USBHS_UGCTRL2_USB2SEL_USB30;
>>>>> +
>>>>> + for (i = 0; i < NUM_USB_CHANNELS; i++) {
>
>>>> Instead of hard coding the number of channels,
>
>>> It's hard coded in the hardware. We can even decrease that number to 2 as
>
>> right, that's why thought dt should have that information.
>
> So you want a dedicated property for that or you meant something else?
>
>>> for the channel #1 we have nothing to do, regardless of whether it's present or
>>> not...
>
>>>> we can model the channels (PHYs) as sub-nodes of the main PHY
>
>>> Hm, I don't think such representation would be adequate: the channels
>>> themselves do not usually correspond to any particular PHY, that's why I used
>>> #phy-cells = <2>.
>
>>>> in dt and use it to create individual PHYs.
>
>>> Well, we probably can... however, I fail to see any immediate gain from
>>> it here...
>>> I have to ask why you've selected this particular driver for such DT
>>> representation experiments, despite it not being the first one supporting
>>> multiple PHYs?
>
>> just that it didn't strike before.. but I think all multiple PHYs should be
>> modelled this way.
>
> I've basically reimplemented the driver to parse the info from the subnodes
> and it's now became larger in size, not smaller. :-/ Overall, I'm not content
> with the changes, nor do I think such a change in the DT representation was a
> great idea...
I've seen someone posted a patch with subnodes and that wasn't looking too bad.
Do you mind posting your patch in the list?
Cheers
Kishon
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
