Re: [PATCH dev-5.0 2/4] mtd: spi-nor: aspeed: clarify 4BYTE address mode mask

["Cédric Le Goater" <clg@kaod.org>]

On 4/19/19 12:27 AM, Milton Miller II wrote:
> About 04/17/2019 09:18AM in some timezone, Cédric Le Goater wrote:
> 
>>Subject: [PATCH dev-5.0 2/4] mtd: spi-nor: aspeed: clarify 4BYTE
>>address mode mask
>>
> 
> Missing  change log here.

This is a oneliner patch.

> 
>>Signed-off-by: Cédric Le Goater <clg@kaod.org <mailto:clg@kaod.org>>
>>---
>> drivers/mtd/spi-nor/aspeed-smc.c | 10 +++++++++-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>
>>diff --git a/drivers/mtd/spi-nor/aspeed-smc.c
>>b/drivers/mtd/spi-nor/aspeed-smc.c
>>index ee3059b27c07..1437732fdea1 100644
>>--- a/drivers/mtd/spi-nor/aspeed-smc.c
>>+++ b/drivers/mtd/spi-nor/aspeed-smc.c
>>@@ -884,7 +884,15 @@ static const uint32_t aspeed_smc_hclk_divs[] = {
>>
>> static u32 aspeed_smc_default_read(struct aspeed_smc_chip *chip)
>> {
>>- return (chip->ctl_val[smc_read] & 0x2000) |
>>+ /*
>>+ * Keep the 4Byte address mode on the AST2400 SPI controller.
>>+ * Other controllers set the 4Byte mode in the CE Control
>>+ * Register
>>+ */
>>+ u32 ctl_mask = chip->controller->info == &spi_2400_info ?
>>+ CONTROL_IO_ADDRESS_4B : 0;
>>+
> 
> I dislike this patch because it violates the data driven model of the types.
> 
> Either a dupicate method should be created or at least a member of the
> type structure should be used instead of a compare to a specific instance.

we could add a ops returning the bitmask. yes. I think this is a efficient
considering the patch is not in mainline and was not reviewed by the Linux
maintainers.

C.


> 
>>+ return (chip->ctl_val[smc_read] & ctl_mask) |
>> (0x00 << 28) | /* Single bit */
>> (0x00 << 24) | /* CE# max */
>> (0x03 << 16) | /* use normal reads */
>>--
>>2.20.1
>>
>>