LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Marek Szyprowski <m.szyprowski@samsung.com>
To: "Måns Rullgård" <mans@mansr.com>
Cc: Peter Chen <peter.chen@nxp.com>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"linux-samsung-soc@vger.kernel.org" 
	<linux-samsung-soc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Markus Reichl <m.reichl@fivetechno.de>,
	Krzysztof Kozlowski <krzk@kernel.org>
Subject: Re: [PATCH v3] usb: core: verify devicetree nodes for USB devices
Date: Fri, 17 May 2019 13:15:24 +0200	[thread overview]
Message-ID: <ccde81df-95e1-7496-52a2-aaf7a303c1fe@samsung.com> (raw)
In-Reply-To: <yw1xzhnqu0r4.fsf@mansr.com>

Hi Måns

On 2019-05-13 12:06, Måns Rullgård wrote:
> Marek Szyprowski <m.szyprowski@samsung.com> writes:
>
>> Hi Peter,
>>
>> On 2019-05-13 11:23, Peter Chen wrote:
>>>> On 2019-05-13 11:00, Peter Chen wrote:
>>>>>> On 2019-05-10 05:10, Peter Chen wrote:
>>>>>>>> Marek Szyprowski <m.szyprowski@samsung.com> writes:
>>>>>>>>> Commit 69bec7259853 ("USB: core: let USB device know device node")
>>>>>>>>> added support for attaching devicetree node for USB devices. The
>>>>>>>>> mentioned commit however identifies the given USB device node only
>>>>>>>>> by the
>>>>>> 'reg'
>>>>>>>>> property in the host controller children nodes. The USB device
>>>>>>>>> node however also has to have a 'compatible' property as described
>>>>>>>>> in Documentation/devicetree/bindings/usb/usb-device.txt. Lack for
>>>>>>>>> the 'compatible' property check might result in assigning a
>>>>>>>>> devicetree node, which is not intended to be the proper node for the given
>>>> USB device.
>>>>>>>>> This is important especially when USB host controller has
>>>>>>>>> child-nodes for other purposes. For example, Exynos EHCI and OHCI
>>>>>>>>> drivers already define child-nodes for each physical root hub port
>>>>>>>>> and assigns respective PHY controller and parameters for them.
>>>>>>>>> Those binding predates support for USB devicetree nodes.
>>>>>>>>>
>>>>>>>>> Checking for the proper compatibility string allows to mitigate
>>>>>>>>> the conflict between USB device devicetree nodes and the bindings
>>>>>>>>> for USB controllers with child nodes. It also fixes the
>>>>>>>>> side-effect of the other commits, like 01fdf179f4b0 ("usb: core:
>>>>>>>>> skip interfaces disabled in devicetree"), which incorrectly
>>>>>>>>> disables some devices on Exynos based boards.
>>>>>>> Hi Marek,
>>>>>>>
>>>>>>> The purpose of your patch is do not set of_node for device under USB
>>>>>>> controller,
>>>>>> right?
>>>>>>
>>>>>> Right.
>>>>>>
>>>>> Do you mind doing it at function exynos_ehci_get_phy of ehci-exynos.c?
>>>> I don't mind fixing it in ehci-exynos, but frankly so far I have no
>>>> idea how to do it.  The problem is that newly created USB devices
>>>> get of-node pointer pointing to a node which if not intended for
>>>> them. How this can be fixed in ehci-exynos?
>>>>
>>>    
>>> Can't be workaround by setting of_node as NULL for EHCI controller or
>>> for PHY node at exynos_ehci_get_phy?
>> Ah, such workaround? I will check, but this will need to be done with
>> care, because have a side effect for other subsystems like regulators or
>> clocks.
>>
>> BTW, What's wrong with proper, full verification of USB device nodes?
> Your approach so far doesn't address the actual problem of a conflict
> between the generic USB DT bindings and those for the Exynos host
> controller.  If you fix that, the validation issue goes away.

Well, the issue caused by the Exynos binding conflict will go away, but 
there still will be a chance that wrong node might be assigned to the 
USB device, especially if partially incorrect data will be stored in the 
device tree. For example, it may happen that on some boards the 
different USB chip is mounted, which require different parameters. The 
code only relies on the reg property and device number, while the 
bindings define compatible string with proper exact vendor/product ids.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


  reply	other threads:[~2019-05-17 11:15 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20190507125630eucas1p1c5fd171a8dc2a6b8eb9dd317fe245f0c@eucas1p1.samsung.com>
2019-05-07 12:56 ` [PATCH] usb: core: verify devicetree nodes for disabled interfaces Marek Szyprowski
2019-05-07 13:24   ` Måns Rullgård
2019-05-08 10:14     ` Marek Szyprowski
     [not found]       ` <CGME20190508104442eucas1p2ebdffa348465f2c28177601014614853@eucas1p2.samsung.com>
2019-05-08 10:44         ` [PATCH v2] " Marek Szyprowski
2019-05-08 11:46           ` Måns Rullgård
2019-05-08 13:49             ` Marek Szyprowski
2019-05-08 15:27               ` Måns Rullgård
     [not found]                 ` <CGME20190509084827eucas1p294962744fe70745c50b69a5349b5de68@eucas1p2.samsung.com>
2019-05-09  8:47                   ` [PATCH v3] usb: core: verify devicetree nodes for USB devices Marek Szyprowski
2019-05-09 18:55                     ` Måns Rullgård
2019-05-10  3:10                       ` Peter Chen
2019-05-10  9:43                         ` Marek Szyprowski
2019-05-13  9:00                           ` Peter Chen
2019-05-13  9:07                             ` Marek Szyprowski
2019-05-13  9:23                               ` Peter Chen
2019-05-13 10:03                                 ` Marek Szyprowski
2019-05-13 10:06                                   ` Måns Rullgård
2019-05-17 11:15                                     ` Marek Szyprowski [this message]
2019-05-13 10:03                           ` Måns Rullgård
2019-05-17 11:18                             ` Marek Szyprowski
2019-05-09 19:04                     ` Tobias Jakobi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ccde81df-95e1-7496-52a2-aaf7a303c1fe@samsung.com \
    --to=m.szyprowski@samsung.com \
    --cc=b.zolnierkie@samsung.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=krzk@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=m.reichl@fivetechno.de \
    --cc=mans@mansr.com \
    --cc=peter.chen@nxp.com \
    --subject='Re: [PATCH v3] usb: core: verify devicetree nodes for USB devices' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).