On 1/26/24 08:46, Conor Dooley wrote:
Yes that is a bit in the controller reg to disable the feature by the driver even the chip have the WP pin connected.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 ;)
Agree it is more on the software for that particular mode.
Well if binding only say 3 possible value, you can not use this property for other value of course. DTS binding check will fail. But agree there is no check on this in the driver side for any integer property.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.
That's fine. I will just change to the boolean.
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.
Will delete.
Thanks, Conor.
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
