linux-kernel - Re: [PATCH v1 1/3] dt-bindings: mmc: Drop unused properties

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: <20230911-implosive-shrill-506d18d9bde2@spud>
Date:   Mon, 11 Sep 2023 17:14:22 +0100
From:   Conor Dooley <conor@...nel.org>
To:     Emil Renner Berthing <emil.renner.berthing@...onical.com>
Cc:     William Qiu <william.qiu@...rfivetech.com>,
        Jessica Clarke <jrtc27@...c27.com>,
        Conor Dooley <conor.dooley@...rochip.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>,
        linux-riscv <linux-riscv@...ts.infradead.org>,
        linux-mmc@...r.kernel.org, Emil Renner Berthing <kernel@...il.dk>,
        Rob Herring <robh+dt@...nel.org>,
        Jaehoon Chung <jh80.chung@...sung.com>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>
Subject: Re: [PATCH v1 1/3] dt-bindings: mmc: Drop unused properties

On Fri, Sep 08, 2023 at 03:32:36PM +0200, Emil Renner Berthing wrote:
> On Fri, 8 Sept 2023 at 12:03, William Qiu <william.qiu@...rfivetech.com> wrote:
> > On 2023/9/2 1:43, Conor Dooley wrote:
> > > On Fri, Sep 01, 2023 at 06:20:38PM +0100, Jessica Clarke wrote:
> > >> On 1 Sep 2023, at 16:42, Conor Dooley <conor@...nel.org> wrote:
> > >> >
> > >> > On Fri, Sep 01, 2023 at 10:33:13AM +0800, William Qiu wrote:
> > >> >>
> > >> >>
> > >> >> On 2023/8/30 16:34, Conor Dooley wrote:
> > >> >>> On Wed, Aug 30, 2023 at 09:29:20AM +0200, Krzysztof Kozlowski wrote:
> > >> >>>> On 30/08/2023 08:50, Conor Dooley wrote:
> > >> >>>>> On Wed, Aug 30, 2023 at 11:18:44AM +0800, William Qiu wrote:
> > >> >>>>>> Due to the change of tuning implementation, it's no longer necessary to
> > >> >>>>>> use the "starfive,sysreg" property in dts, so drop the relevant
> > >> >>>>>> description in dt-bindings here.
> > >> >>>>>
> > >> >>>>> How does changing your software implantation invalidate a description of
> > >> >>>>> the hardware?
> > >> >>>>>
> > >> >>>>
> > >> >>>> Which is kind of proof that this syscon was just to substitute
> > >> >>>> incomplete hardware description (e.g. missing clocks and phys). We
> > >> >>>> should have rejected it. Just like we should reject them in the future.
> > >> >>>
> > >> >>> :s I dunno what to do with this... I'm inclined to say not to remove it
> > >> >>> from the binding or dts at all & only change the software.
> > >> >>>
> > >> >>>> There are just few cases where syscon is reasonable. All others is just
> > >> >>>> laziness. It's not only starfivetech, of course. Several other
> > >> >>>> contributors do the same.
> > >> >>>
> > >> >>> I'm not sure if laziness is fair, lack of understanding is usually more
> > >> >>> likely.
> > >> >>
> > >> >> For this, I tend to keep it in binding, but remove it from required. Because
> > >> >> we only modify the tuning implementation, it doesn't mean that this property
> > >> >> need to be removed, it's just no longer be the required one.
> > >> >
> > >> > Please only remove it from required if the current driver doesn't break
> > >> > if the regmap is removed.
> > >>
> > >> Either way please make sure the documentation clearly states “never use
> > >> this, if you’re using it you’re doing it wrong, this only exists
> > >> because it was wrongly used in the past”. Otherwise people writing
> > >> drivers for other OSes will probably use it too thinking they need to.
> > >
> > > Maybe we should just delete it if the impact is going to be negligible,
> > > sounds like you're not using it in FreeBSD, which was part of what I was
> > > worried about. Guess it depends on what Emil & the distro heads think.
> > Hi Conor,
> >
> > After discussing it with our colleagues, we decided that deleting it was the best
> > course of action. Since there will no longer be a related implementation of
> > "starfive,sysreg" in future drivers, even if the dt-binding is described, it will
> > be "never use", so I think it should be deleted.
> >
> > What do you think?
> 
> The device tree should be a description of the hardware and there
> really is a 'u0_sdio_data_strobe_phase_ctrl' field in the sysreg
> registers[1] on the JH7110 that seems to do _something_ related to the
> sdio interface. So I don't think the fact that the Linux driver no
> longer uses it is a good reason to remove it, but if there are some
> other pragmatic reasons to do so then I'm fine with it. Removing it
> from the list of required properties should be fine though.

SGTM. Can you update the patch to do that please William?

Thanks,
Conor.

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)