[linux-sunxi] [PATCH] musb: sunxi: Ignore VBus errors in host-only mode

[Hans de Goede]

Hi,

On 14-09-15 18:58, Bin Liu wrote:
> Hi,
>
> On Mon, Sep 14, 2015 at 9:59 AM, Hans de Goede <hdegoede at redhat.com> wrote:
>> Hi,
>>
>>
>> On 14-09-15 16:44, Bin Liu wrote:
>>>
>>> Hi,
>>>
>>> On Thu, Sep 10, 2015 at 1:38 PM, Hans de Goede <hdegoede at redhat.com>
>>> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 10-09-15 20:30, Maxime Ripard wrote:
>>>>>
>>>>>
>>>>> On Thu, Sep 10, 2015 at 08:23:23PM +0200, Hans de Goede wrote:
>>>>>>
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On 04-09-15 08:43, Olliver Schinagl wrote:
>>>>>>>
>>>>>>>
>>>>>>> Hey Hans,
>>>>>>>
>>>>>>> On 07-08-15 10:45, Olliver Schinagl wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> <snip>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> If you change the dr_mode to host then you _must_ also remove any
>>>>>>>>> id_det and vbus_det
>>>>>>>>> gpio settings from the usb_phy node in the dts, as the sun4i phy
>>>>>>>>> code
>>>>>>>>> detects
>>>>>>>>> host vs otg mode by checking for the presence of these.
>>>>>>>>
>>>>>>>>
>>>>>>>> Yes, this fixes it and makes it work. Thanks.
>>>>>>>>
>>>>>>> I've been going back to this and am wondering if this is something I
>>>>>>> can
>>>>>>> look into to fix properly? E.g. if the dts sets dr_mode = host, can we
>>>>>>> simply ignore the pins and treat them as unset?
>>>>>>
>>>>>>
>>>>>>
>>>>>> AFAIK you cannot unset something in dts. The only solution I
>>>>>> can comeup with is to add a dr_mode argument to the phy like
>>>>>> we already have for the otg controller itself.
>>>>>>
>>>>>> This is something which we likely need to do anyways to add
>>>>>> support for peripheral only mode, which we seem to need for
>>>>>> some "hdmi sticks".
>>>>>
>>>>>
>>>>>
>>>>> I haven't really followed the rest of the discussion, so sorry if you
>>>>> already talked about that, but why can't you just set the dr_mode to
>>>>> peripheral in such a case?
>>>>
>>>>
>>>>
>>>> This is about the usbphy code not the musb-controller code, which are
>>>> 2 different dts nodes, atm only the musb-controller node has a
>>>> dr_mode property, and the phy code decides between host-only
>>>> and otg mode based on whether an id pin is assigned or not.
>>>>
>>>> My proposal is to get rid of the id-pin hack to determine the mode
>>>> and add a dr_mode property to the usbphy dts node.
>>>
>>>
>>> I would try to avoid adding dr_mode in the usbphy node if possible,
>>> since it is already specified in the controller node.
>>>
>>> Since the phy node is referenced in the controller node, is it
>>> possible that the phy driver looks up the controller of properties to
>>> find its dr_mode setting?
>>
>>
>> That is possible, but it very much goes against how devicetree normally
>> works, where every node is more or less a standalone unit which
>> contains enough info for the associated driver to do its work.
>
> I guess you are right, but duplicating dr_mode would cause more
> trouble for end user.

The user already needs to set up regulators, vbus-det and id-det
for otg mode in the usbphy node. Although there are 2 nodes in dts /
2 separate hardware blocks involved in reality the 2 are closely
related and the user already must take care to have the settings match.

Besides that writing dts files is not something which end users do,
so a normal user will never see this.

> Felipe, could you please give your comments on this issue? I have to
> do the similar change for musb_dsps.
>
>>
>> Currently there is no link from the phy node to the controller node
>> (only the other way around) and adding such a link requires more code
>> then simple having a duplicate dr_mode.
>
> I guess I did not make my previous comment clearly, we don't need to
> add the link from the phy node to the controller node. We don't need
> to change the current dts, just in the phy driver to look up dr_mode
> of the controller node, if possible.

And how does the phy code now where the controller node lives without
having a link to it in its node? Hardcode the full path to the
controller node ? That is both ugly and error prone.

Regards,

Hans