linux-kernel - Re: [PATCH RESEND v4 00/37] mtd: st_spi

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] [day] [month] [year] [list]

Message-ID: <20140214104655.GE9462@lee--X1>
Date:	Fri, 14 Feb 2014 10:46:55 +0000
From:	Lee Jones <lee.jones@...aro.org>
To:	Angus Clark <Angus.Clark@...com>,
	Brian Norris <computersforpeace@...il.com>
Cc:	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	linus.walleij@...aro.org, dwmw2@...radead.org,
	linux-mtd@...ts.infradead.org, computersforpeace@...il.com,
	DCG_UPD_stlinux_kernel@...t.st.com, olivier.clergeaud@...com
Subject: Re: [PATCH RESEND v4 00/37] mtd: st_spi_fsm: Add new driver

> Sorry for the delay.  I have now had a quick look through the patches.  Just a
> couple of points :-)
> 
> * stfsm_probe(): stfsm_fetch_platform_configs() needs to be called *before*
> config() -- config() is based on platform capabilities.  Conceptually,
> stfsm_fetch_platform_configs() should be called before stfsm_jedec_probe(), and
> FLASH_FLAG_32BIT_ADDR should be moved out of stfsm_fetch_platform_configs(),
> placed just after stfsm_jedec_probe() but before config().
> 
> * fsm_wait_busy(): logic not quite correct if we get a P_ERR or E_ERR error
> after a timeout.  I am also not sure about returning (uint8_t)-EIO.  For what
> its worth, this is what I did in response to Brian's comment about the race
> condition:

Thanks Angus.

Brain,

If I fix up the review comments from  Angus', Ludovic and Christophe
are you happy to finally accept the driver? Or do you have any final
review comments that you wish me to apply for v5?

> On 01/23/2014 10:30 AM, Lee Jones wrote:
> > Version 4:
> >   Tended to Brian's review comments
> >     - Checkpatch acceptance
> >     - MODULE_DEVICE_TABLE() name slip correction
> >     - Timeout issue(s) resolved
> >     - Potential infinite loop mitigated
> >     - Code clarity suggests heeded
> >     - Duplication with MTD core code removed
> >     - Upgraded to using ROUND_UP() helper
> >     - Moved non-shared header code into main driver
> >     - Relocated dynamic msg sequence stores into main struct
> >     - Averted adaption of static (table) data
> >     - Basic whitespace/spelling/data type/dev_err suggestions applied
> >   
> > Version 3:
> >   Okay, this thing should be fully functional now. Identify a chip
> >   based on it's JEDEC ID, Read, Write, Erase (all or by sector).
> >   Support for various chip quirks added too.
> > 
> > Version 2:
> >   The first bunch of these patches have been on the MLs before, but
> >   didn't receive a great deal of attention for the most part. We are
> >   a little more featureful this time however. We can now successfully
> >   setup and configure the N25Q256. We still can't read/write/erase
> >   it though. I'll start work on that next week and will provide it in
> >   the next instalment.
> > 
> > Version 1:
> >   First stab at getting this thing Mainlined. It doesn't do a great deal
> >   yet, but we are able to initialise the device and dynamically set it up
> >   correctly based on an extracted JEDEC ID.
> > 
> >  Documentation/devicetree/bindings/mtd/st-fsm.txt |   26 ++
> >  arch/arm/boot/dts/stih416-b2105.dts              |   14 +
> >  arch/arm/boot/dts/stih416-pinctrl.dtsi           |   12 +
> >  drivers/mtd/devices/Kconfig                      |    8 +
> >  drivers/mtd/devices/Makefile                     |    1 +
> >  drivers/mtd/devices/serial_flash_cmds.h          |   81 ++++
> >  drivers/mtd/devices/st_spi_fsm.c                 | 2124 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 7 files changed, 2266 insertions(+)
> > 
> > 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/