[PATCH v3 08/10] phy: socionext: Add UniPhier USB3 PHY driver

[Kunihiko Hayashi]

Hi Marek,

On 2023/02/18 4:58, Marek Vasut wrote:
> On 2/16/23 17:14, Kunihiko Hayashi wrote:
>> Hi Marek,
> 
> Hello Hayashi-san,
> 
>> Sorry for late reply.
>>
>> On 2023/02/14 6:06, Marek Vasut wrote:
>>> On 2/13/23 04:08, Kunihiko Hayashi wrote:
>>>> Hi Marek,
>>>
>>> Hello Hayashi-san,
>>>
>>
>> [...]
>>
>>>> I think so, however, when I added .exit() and executed "usb stop;usb
>>>> start",
>>>> unfortunately the command got stuck.
>>>>
>>>> Currently uniphier clk doesn't support CLK_CCF and can't be nested.
>>>> I think more changes are needed.
>>>
>>> Do you know where exactly the system hangs ?
>>
>> Yes.
>>
>> The controller-reset driver controls the clock/reset for the glue
>> in probe(). The phy driver controls the clock/reset for the glue and
>> the phy in init(). There is an inconsistency.
>>
>> When submitting "usb stop", the phy driver disables all the clock/reset
>> in exit().
>>
>> When submitting "usb start" again, the controller-reset driver accesses
>> the glue register without enabling the clock/reset, and the system hangs.
> 
> Shouldn't the PHY enable its own set of clock, while the controller
> should only enable its own (different) set of clock ?

PHY has this own clock/reset, however, PHY is in the glue logic.
So accessing to PHY also needs to enable the glue clock
(same as the controller clock).

> If it is the same clock and there is no refcounting, then maybe what you
> should do for now is enable/disable all the clock in the controller
> driver only, and deal with the PHY once CCF with refcounting is in place ?

PHY needs the glue(controller) clock and the phy clock.
After all, it's wrong to control all the clocks/resets at the same time.

In this time, I don't event think about CCF (sorry to confuse you).

>>> Do you expect to add the CCF support ?
>>
>> Yes, first I thought the clock needed the CCF support to count the clock
>> function calls, but there is no support to count the reset function calls.
>> I need another solution.
>>
>> Currently the phy driver controls all the clock/reset in init() and
>> exit(),
>> however, it should control the phy clock/reset only.
> 
> If that's possible on this hardware, that would be good, let controller
> control its clock and PHY control its (different) clock.

Yes. I'll separate these controls.

>>> If so, then add at least a FIXME code comment that the exit callback
>>> must be implemented, so that this is not forgotten once CCF is in place.
>>> I don't want this series to grow much further into some "rework
>>> everything at once" thing.
>>
>> As above, I should put the glue clock/reset into probe(),
>> and the phy clock/reset into init() without using bulk function.
> 
> OK

Thank you,

---
Best Regards
Kunihiko Hayashi