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@xxxxxxxxxxxx> > > > 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.
Attachment:
signature.asc
Description: PGP signature
