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

[Andrew Jeffery]

On Fri, 19 Apr 2019, at 15:39, Cédric Le Goater wrote:
> 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.

Yes, I think the comment more than made up for the lack of a changelog.

Easy enough to view both together, and I find that acceptable for now.

Andrew

> 
> > 
> >>Signed-off-by: Cédric Le Goater <clg at kaod.org <mailto:clg at 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
> >>
> >>
> 
>