| Message ID | b65240ff-7e28-5ecb-0670-abfca871246b@gmail.com |
|---|---|
| State | Accepted |
| Commit | 9e276de6a367cde07c1a63522152985d4e5cca8b |
| Headers | show |
Am 17.08.2016 um 21:09 schrieb Heiner Kallweit: > Consider a message size limit when calculating the maximum amount > of data that can be read. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > v2: > - use the new function spi_max_message_size > --- > drivers/mtd/devices/m25p80.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c > index 9cf7fcd..16a7df2 100644 > --- a/drivers/mtd/devices/m25p80.c > +++ b/drivers/mtd/devices/m25p80.c > @@ -172,7 +172,8 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len, > > t[1].rx_buf = buf; > t[1].rx_nbits = m25p80_rx_nbits(nor); > - t[1].len = min(len, spi_max_transfer_size(spi)); > + t[1].len = min3(len, spi_max_transfer_size(spi), > + spi_max_message_size(spi) - t[0].len); > spi_message_add_tail(&t[1], &m); > > ret = spi_sync(spi, &m); > Can we still get this into 4.9 ? The other two patches of the series have been applied by Mark already and there's a dependency for messages >64k on fsl-espi.
Le 17/08/2016 à 21:09, Heiner Kallweit a écrit : > Consider a message size limit when calculating the maximum amount > of data that can be read. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Reviewed-by: Cyrille Pitchen <cyrille.pitchen@atmel.com> > --- > v2: > - use the new function spi_max_message_size > --- > drivers/mtd/devices/m25p80.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c > index 9cf7fcd..16a7df2 100644 > --- a/drivers/mtd/devices/m25p80.c > +++ b/drivers/mtd/devices/m25p80.c > @@ -172,7 +172,8 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len, > > t[1].rx_buf = buf; > t[1].rx_nbits = m25p80_rx_nbits(nor); > - t[1].len = min(len, spi_max_transfer_size(spi)); > + t[1].len = min3(len, spi_max_transfer_size(spi), > + spi_max_message_size(spi) - t[0].len); > spi_message_add_tail(&t[1], &m); > > ret = spi_sync(spi, &m); >
Le 17/08/2016 à 21:09, Heiner Kallweit a écrit : > Consider a message size limit when calculating the maximum amount > of data that can be read. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Applied to git://github.com/spi-nor/linux.git Thanks! > --- > v2: > - use the new function spi_max_message_size > --- > drivers/mtd/devices/m25p80.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c > index 9cf7fcd..16a7df2 100644 > --- a/drivers/mtd/devices/m25p80.c > +++ b/drivers/mtd/devices/m25p80.c > @@ -172,7 +172,8 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len, > > t[1].rx_buf = buf; > t[1].rx_nbits = m25p80_rx_nbits(nor); > - t[1].len = min(len, spi_max_transfer_size(spi)); > + t[1].len = min3(len, spi_max_transfer_size(spi), > + spi_max_message_size(spi) - t[0].len); > spi_message_add_tail(&t[1], &m); > > ret = spi_sync(spi, &m); >
On 12/23/2016 04:33 PM, Cyrille Pitchen wrote: > Le 17/08/2016 à 21:09, Heiner Kallweit a écrit : >> Consider a message size limit when calculating the maximum amount >> of data that can be read. >> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > > Applied to git://github.com/spi-nor/linux.git Is that the concensus now that we should fix controller crappiness on driver level ? Or did I miss the point of this patch ? > Thanks! > >> --- >> v2: >> - use the new function spi_max_message_size >> --- >> drivers/mtd/devices/m25p80.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c >> index 9cf7fcd..16a7df2 100644 >> --- a/drivers/mtd/devices/m25p80.c >> +++ b/drivers/mtd/devices/m25p80.c >> @@ -172,7 +172,8 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len, >> >> t[1].rx_buf = buf; >> t[1].rx_nbits = m25p80_rx_nbits(nor); >> - t[1].len = min(len, spi_max_transfer_size(spi)); >> + t[1].len = min3(len, spi_max_transfer_size(spi), >> + spi_max_message_size(spi) - t[0].len); >> spi_message_add_tail(&t[1], &m); >> >> ret = spi_sync(spi, &m); >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-spi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Hello, On 23 December 2016 at 19:53, Marek Vasut <marek.vasut@gmail.com> wrote: > On 12/23/2016 04:33 PM, Cyrille Pitchen wrote: >> Le 17/08/2016 à 21:09, Heiner Kallweit a écrit : >>> Consider a message size limit when calculating the maximum amount >>> of data that can be read. >>> >>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> >> Applied to git://github.com/spi-nor/linux.git > > Is that the concensus now that we should fix controller crappiness on > driver level ? Or did I miss the point of this patch ? > In general most controllers do have some limit on the amount of data they can transfer at once. For some you can apply workaround at controller driver level and for some it's not practical. Also for some the limit is lower so it is more likely hit. I guess you can call controllers that have both of these limitations crappy but that's what's in the silicone out there. If you have better solution to the problem please share it in detail. Thanks Michal
On 12/23/2016 10:04 PM, Michal Suchanek wrote: > Hello, Hi, > On 23 December 2016 at 19:53, Marek Vasut <marek.vasut@gmail.com> wrote: >> On 12/23/2016 04:33 PM, Cyrille Pitchen wrote: >>> Le 17/08/2016 à 21:09, Heiner Kallweit a écrit : >>>> Consider a message size limit when calculating the maximum amount >>>> of data that can be read. >>>> >>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >>> >>> Applied to git://github.com/spi-nor/linux.git >> >> Is that the concensus now that we should fix controller crappiness on >> driver level ? Or did I miss the point of this patch ? >> > > In general most controllers do have some limit on the amount of data > they can transfer at once. My observations suggest otherwise. > For some you can apply workaround at controller driver level and for > some it's not practical. > > Also for some the limit is lower so it is more likely hit. > > I guess you can call controllers that have both of these limitations > crappy but that's what's in the silicone out there. > > If you have better solution to the problem please share it in detail. I don't have a solution, but I am quite sure that if we now set a precedent that each and every device driver should take care of each and every controller's quirk, we have a lot of unpleasant work ahead of us and it will be quite the maintanance burden. The separations between device driver and controller driver has been weakened and I don't think that's good.
Am 24.12.2016 um 11:20 schrieb Marek Vasut: > On 12/23/2016 10:04 PM, Michal Suchanek wrote: >> Hello, > > Hi, > >> On 23 December 2016 at 19:53, Marek Vasut <marek.vasut@gmail.com> wrote: >>> On 12/23/2016 04:33 PM, Cyrille Pitchen wrote: >>>> Le 17/08/2016 à 21:09, Heiner Kallweit a écrit : >>>>> Consider a message size limit when calculating the maximum amount >>>>> of data that can be read. >>>>> >>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >>>> >>>> Applied to git://github.com/spi-nor/linux.git >>> >>> Is that the concensus now that we should fix controller crappiness on >>> driver level ? Or did I miss the point of this patch ? >>> >> >> In general most controllers do have some limit on the amount of data >> they can transfer at once. > > My observations suggest otherwise. > >> For some you can apply workaround at controller driver level and for >> some it's not practical. >> >> Also for some the limit is lower so it is more likely hit. >> >> I guess you can call controllers that have both of these limitations >> crappy but that's what's in the silicone out there. >> >> If you have better solution to the problem please share it in detail. > > I don't have a solution, but I am quite sure that if we now set a > precedent that each and every device driver should take care of each and > every controller's quirk, we have a lot of unpleasant work ahead > of us and it will be quite the maintanance burden. The separations > between device driver and controller driver has been weakened and I > don't think that's good. > The controller driver API has been extended to allow exposing certain types of controller limits (max tranfer size, max message size) to upper layers. And controller drivers rely on upper layers considering this information. W/o this patch certain combinations of protocol and controller drivers (e.g. m25p80 + fsl-espi) don't work for big transfer / message sizes. The (most likely more ugly) alternative is controller drivers implementing protocol-specific hacks to deal with messages exceeding controller limits. One example was the fsl-espi drivers before the recent refactoring. It assumed all big messages to be m25p80.
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c index 9cf7fcd..16a7df2 100644 --- a/drivers/mtd/devices/m25p80.c +++ b/drivers/mtd/devices/m25p80.c @@ -172,7 +172,8 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len, t[1].rx_buf = buf; t[1].rx_nbits = m25p80_rx_nbits(nor); - t[1].len = min(len, spi_max_transfer_size(spi)); + t[1].len = min3(len, spi_max_transfer_size(spi), + spi_max_message_size(spi) - t[0].len); spi_message_add_tail(&t[1], &m); ret = spi_sync(spi, &m);
Consider a message size limit when calculating the maximum amount of data that can be read. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- v2: - use the new function spi_max_message_size --- drivers/mtd/devices/m25p80.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)