linux-kernel - Re: [PATCH v3 01/10] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs

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]

Date: Fri, 26 Jan 2024 16:46:46 +0000
From: Conor Dooley <conor@...nel.org>
To: William Zhang <william.zhang@...adcom.com>
Cc: David Regan <dregan@...adcom.com>, dregan@...l.com,
	miquel.raynal@...tlin.com, richard@....at, vigneshr@...com,
	robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
	conor+dt@...nel.org, computersforpeace@...il.com,
	kdasu.kdev@...il.com, linux-mtd@...ts.infradead.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	joel.peshkin@...adcom.com, tomer.yacoby@...adcom.com,
	dan.beygelman@...adcom.com, anand.gore@...adcom.com,
	kursad.oney@...adcom.com, florian.fainelli@...adcom.com,
	rafal@...ecki.pl, bcm-kernel-feedback-list@...adcom.com,
	andre.przywara@....com, baruch@...s.co.il,
	linux-arm-kernel@...ts.infradead.org, dan.carpenter@...aro.org
Subject: Re: [PATCH v3 01/10] dt-bindings: mtd: brcmnand: Updates for bcmbca
 SoCs

On Thu, Jan 25, 2024 at 05:55:29PM -0800, William Zhang wrote:
> On 1/25/24 11:51, Conor Dooley wrote:
> > On Wed, Jan 24, 2024 at 07:01:48PM -0800, William Zhang wrote:
> > > On 1/24/24 09:24, Conor Dooley wrote:
> > > > On Tue, Jan 23, 2024 at 07:04:49PM -0800, David Regan wrote:
> > > > > From: William Zhang <william.zhang@...adcom.com>

> > > And the reason I keep it as int is because there could be a potential value
> > > of 2 for another mode that the current driver could set the wp_on to.
> > 
> > Can you explain, in driver independent terms, what this other mode has
> > to do with how the WP is connected?
> > Either you've got the standard scenario where apparently "NAND_WPb" and
> > "WP_L" are connected and another situation where they are not.
> > Is there another pin configuration in addition to that, that this 3rd
> > mode represents?
> > 
> The 3rd mode is WP pin connected but wp feature is disabled in the
> controller.

What does "disabled" mean? The controller itself is not capable of using
the WP pin because of hardware modifications? Or there's a bit in a
register that disables it and that bit has been set by software?

If it is a hardware difference for that controller, why does it not have
a dedicated compatible? If it's a software decision, then it shouldn't
be in the DT in the first place ;)

We've gone back here a bunch trying to figure stuff out, and you give me
a vague one sentence answer. Please.

> > > I
> > > don't see it is being used in BCMBCA product but I am not sure about other
> > > SoC family. So I don't want to completely close the door here just in case.
> > 
> > If you ever need it, you could have a "brcm,wp-in-wacky-configuration"
> > property that indicates that the hardware is configured in that way
> > (providing that this hardware configuration is not just "NAND_WPb" and
> > "WP_L" pins connected. The property can very easily be made mutually
> > exclusive with "brcm,wp-not-connected" iff it ever comes to be.
> Yes we could have a new property but if we can have a single int property to
> cover all, IMHO it is better to just have one property as these three modes
> are related. I would really like to have it as an int property to keep
> things simple.

It is "better" because it is easier for you perhaps, but ripe for abuse.

> But if you think this is absolutely against the dts rule,  I will switch to
> the "brcm,wp-not-connected" boolean property as you suggested.

I'll answer this when you describe the mode better, right now I cannot
really comment, but I have not yet seen a justification for the non-flag
version of the property. Even if there's a justification for documenting
that other mode in the DT, I'm likely to still think boolean properties
are a better fit.

> > > > > If ecc
> > > > > +          strength and spare area size are set by nand-ecc-strength
> > > > > +          and brcm,nand-oob-sector-size in the dts, these settings
> > > > > +          have precedence and override this flag.
> > > > 
> > > > Again, IMO, this is not for the binding to decide. That is a decision
> > > > for the operating system to make.
> > > > 
> > > Again this is just for historic reason and BCMBCA as late player does not
> > > want to break any existing dts setting.  So I thought it is useful to
> > > explain here.  I can remove this text if you don't think it should be in the
> > > binding document.
> > 
> > I don't, at least not in that form. I think it is reasonable to explain
> > why these values are better though.
> I understand the decision is for OS/driver to make. If we were to write
> another driver for other OS, the same precedence will apply too so the
> binding works the same way across all the OS. So I am not sure what better
> explanation or form I can put up here. I am open to any suggestion or I just
> delete it.

I would just delete it tbh.

Thanks,
Conor.

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