[U-Boot] [PATCH 03/18] sf: fix sf probe

[Tom Rini]

On Mon, Aug 04, 2014 at 05:19:28PM +0300, Nikita Kiryanov wrote:
> 
> 
> On 04/08/14 17:02, Tom Rini wrote:
> >On Mon, Aug 04, 2014 at 04:45:57PM +0300, Nikita Kiryanov wrote:
> >>
> >>
> >>On 04/08/14 16:10, Marek Vasut wrote:
> >>>On Monday, August 04, 2014 at 02:48:54 PM, Nikita Kiryanov wrote:
> >>>>Hi Marek,
> >>>>
> >>>>On 03/08/14 16:46, Marek Vasut wrote:
> >>>>>On Sunday, August 03, 2014 at 09:34:33 AM, Nikita Kiryanov wrote:
> >>>>>>MXC SPI driver has a feature whereas a GPIO line can be used as a CS
> >>>>>>signal. This is set up by joining the CS and GPIO values into a single
> >>>>>>value using (cs | gpio << 8), and passing it off as a CS value. This
> >>>>>>breaks the sf probe command, because it is no longer possible to invoke
> >>>>>>it as sf probe <cs>. Instead, the user must use sf probe <cs | gpio <<
> >>>>>>8>.
> >>>>>>
> >>>>>>Fix this by introducing a new board function: board_spi_cs_gpio().
> >>>>>>When called, board_spi_cs_gpio() will return the gpio number for the
> >>>>>>cs value it is given.
> >>>>>>
> >>>>>>Cc: Jagannadha Sutradharudu Teki <jagannadh.teki at gmail.com>
> >>>>>>Cc: Eric Nelson <eric.nelson at boundarydevices.com>
> >>>>>>Cc: Eric Benard <eric at eukrea.com>
> >>>>>>Cc: Fabio Estevam <fabio.estevam at freescale.com>
> >>>>>>Cc: Tim Harvey <tharvey at gateworks.com>
> >>>>>>Cc: Stefano Babic <sbabic at denx.de>
> >>>>>>Cc: Tom Rini <trini at ti.com>
> >>>>>>Signed-off-by: Nikita Kiryanov <nikita at compulab.co.il>
> >>>>>
> >>>>>Just curious, but is this fixing generic SF code or MXC SPI driver ? I'd
> >>>>>think the later, but it's not obvious from neither the description nor
> >>>>>the subject. I don't quite understand the problem that you're trying to
> >>>>>fix either, what happened, did the user command interface change ?
> >>>>
> >>>>The U-Boot shell command "sf probe" can accept a chip select value, but
> >>>>if the SPI device on the other end requires an active chip-select over
> >>>>multiple transactions (achieved in the MXC SPI driver using a GPIO),
> >>>>simply typing something like "sf probe 0" will not work.
> >>>
> >>>Why not ?
> >>>
> >>>>This is because whatever the user passes as chip select is propagated
> >>>>to the driver, and the driver expects this value to have GPIO
> >>>>information. So for example, if IMX_GPIO_NR(2, 30) is used to force
> >>>>active chip select 0, then instead of "sf probe 0" the user will have
> >>>>to type "sf probe 15872".
> >>>
> >>>You mean sf probe 0:15872 , right ?
> >>
> >>It's the same thing:
> >>sf probe [[bus:]cs] [hz] [mode]
> >>
> >>The point is that cs 0 has to be represented as "15872", instead of "0".
> >
> >Eeep.  That seems very likely to be gotten incorrect by users.
> >
> >Can we do something like:
> >mxc_spi.c:
> >__weak int board_map_spi_cs_value(int desired_cs) { return -EINVAL; }
> >
> >fooboard.c:
> >board_map_spi_cs_value(int desired_cs) {
> >   if (desired_cs == 0)
> >     return IMX_GPIO_NR(2, 30);
> >   else
> >     return -EINVAL;
> >}
> 
> That's pretty much what the patch does.
> 
> >
> >I think it'll be very bad if the user has to type 'sf probe 0:15872' or
> >'sf probe 15872' since that's a programming detail rather than saying
> >bank 2, gpio 30 (which I assume is what IMX_GPIO_NR means).
> >
> 
> Agreed.

OK, it's just not clear from the commit messages then that 'sf probe 0'
is still the correct thing to do for the boards and that behind the
scenes we correct things to '15872'

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140804/eb0b50ca/attachment.pgp>