linux-kernel - Re: [PATCH v3 0/9] Add the Renesas USBF controller support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL_JsqLjD=DbKJRrmn2GmbAwmVyE=gVcz-fZfNJ_xAup6GReSA@mail.gmail.com>
Date:   Thu, 8 Dec 2022 14:44:31 -0600
From:   Rob Herring <robh+dt@...nel.org>
To:     Herve Codina <herve.codina@...tlin.com>
Cc:     Geert Uytterhoeven <geert+renesas@...der.be>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Magnus Damm <magnus.damm@...il.com>,
        Gareth Williams <gareth.williams.jx@...esas.com>,
        linux-renesas-soc@...r.kernel.org, linux-clk@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-usb@...r.kernel.org,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Miquel Raynal <miquel.raynal@...tlin.com>
Subject: Re: [PATCH v3 0/9] Add the Renesas USBF controller support

On Thu, Dec 8, 2022 at 2:24 AM Herve Codina <herve.codina@...tlin.com> wrote:
>
> Hi Rob,
>
> On Wed, 7 Dec 2022 16:19:42 -0600
> Rob Herring <robh+dt@...nel.org> wrote:
>
> > On Wed, Dec 7, 2022 at 10:24 AM Herve Codina <herve.codina@...tlin.com> wrote:
> > >
> > > Hi,
> > >
> > > This series add support for the Renesas USBF controller (USB Device
> > > Controller) available in the Renesas RZ/N1 SoC.
> > >
> > > Based on previous review:
> > >   https://lore.kernel.org/all/20221114111513.1436165-3-herve.codina@bootlin.com/
> > >
> > > A new strategy is proposed to handle the H2MODE bit from CFG_USB
> > > register compared to the previous versions on the series. As a
> > > reminder, H2MODE bit allows to configure the internal USB Port
> > > interface for two hosts or one host and one device.
> >
> > Is this case any different from all the phandle properties we have in
> > bindings that point to some misc registers somewhere else you need to
> > poke? If so, I'm not really a fan of duplicating the information.
>
> Our case is that there is a bit in a register that affect several
> devices. This bit must be set before the devices are started.
> If this bit is changed while affected devices are running, system
> hangs can occurs (datasheet).
>
> So, in order to do that we need the device in charge to set
> this bit (sysctrl) to set this bit before other devices (USBF
> and PCI bridge) were started.

That sounds like you just need some platform level initialization and
you are working around the desire to not have platform level
initialization.

Why doesn't/can't the bootloader initialize this? Seems like it might
want to use PCI or USB too.

> At sysctrl level, the bit is set during the probe() call.
> The property 'depends-on' aim is to ensure the probe() calls
> order between provider (sysctrl) and consumers (USBF and PCI
> bridge).
>
> regmap and syscon are used to export registers from one device
> to an other and the probe() calls order is not ensured by the
> core or regmap infrastructure. Indeed, the regmap provider
> probe() will not be called if the regmap provider was not probed
> before the consumer ask for the regmap.
>   https://elixir.bootlin.com/linux/latest/source/drivers/mfd/syscon.c#L152
>   https://elixir.bootlin.com/linux/latest/source/drivers/mfd/syscon.c#L43
> No specific action synchronisation are done with regmap/syscon
> other than the regmap creation itself.

Oh right. That's in place of course to avoid probe ordering issues...

> I don't think the regmap/syscon will help in our case.
>
> >
> > We also have cases of of_find_compatible_node(NULL, NULL,
> > "foo-bar-syscon") which is a dependency expressed in the driver, but
> > not DT. In either case, adding 'depends-on' would be an ABI break as
> > you are requiring a DT change.
>
> In order to avoid the DT change, I can keep the 'depends-on'
> optional in the PCI bridge binding.
> This will be functionnal as sysctrl is already used in this node
> (power-domain = <&sysctrl>). The relationship is already present
> with this power-domain link.
>
> If ok, I will do this change in v4 series.

I agree with Geert that this shouldn't be needed.

Rob