mbox series

[v2,RESEND,0/9] Some fixes for spi-s3c64xx

Message ID 20201001152148.29747-1-l.stelmach@samsung.com (mailing list archive)
Headers show
Series Some fixes for spi-s3c64xx | expand

Message

Łukasz Stelmach Oct. 1, 2020, 3:21 p.m. UTC
This is a series of fixes created during porting a device driver (these
patches will be released soon too) for an SPI device to the current kernel.

The two most important are

  spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and s3c64xx_enable_datapath()
  spi: spi-s3s64xx: Add S3C64XX_SPI_QUIRK_CS_AUTO for Exynos3250

Without them DMA transfers larger than 512 bytes from the SPI controller
would fail.

Łukasz Stelmach (9):
  spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and
    s3c64xx_enable_datapath()
  spi: spi-s3s64xx: Add S3C64XX_SPI_QUIRK_CS_AUTO for Exynos3250
  spi: spi-s3c64xx: Check return values
  spi: spi-s3c64xx: Report more information when errors occur
  spi: spi-s3c64xx: Rename S3C64XX_SPI_SLAVE_* to S3C64XX_SPI_CS_*
  spi: spi-s3c64xx: Fix doc comment for struct s3c64xx_spi_driver_data
  spi: spi-s3c64xx: Ensure cur_speed holds actual clock value
  spi: spi-s3c64xx: Increase transfer timeout
  spi: spi-s3c64xx: Turn on interrupts upon resume

 drivers/spi/spi-s3c64xx.c | 111 +++++++++++++++++++++++++++-----------
 1 file changed, 79 insertions(+), 32 deletions(-)

Changes in v2:
  - added missing commit descriptions
  - added spi: spi-s3c64xx: Ensure cur_speed holds actual clock value
  - implemented error propagation in
      spi: spi-s3c64xx: Check return values
  - rebased onto v5.9-rc1 which contains
      spi: spi-s3c64xx: Add missing entries for structs 's3c64xx_spi_dma_data' and 's3c64xx_spi_dma_data'

Comments

Mark Brown Oct. 1, 2020, 4:13 p.m. UTC | #1
On Thu, Oct 01, 2020 at 05:21:39PM +0200, Łukasz Stelmach wrote:
> This is a series of fixes created during porting a device driver (these
> patches will be released soon too) for an SPI device to the current kernel.

There appeared to be a number of outstanding review comments (misleading
commit message on patch 7, some concerns about the non-CMU case), please
address those.

Please don't send new patches in reply to old ones, it buries them in
threads and can make it hard to follow what's going on.
Łukasz Stelmach Oct. 1, 2020, 6:23 p.m. UTC | #2
It was <2020-10-01 czw 17:13>, when Mark Brown wrote:
> On Thu, Oct 01, 2020 at 05:21:39PM +0200, Łukasz Stelmach wrote:
>> This is a series of fixes created during porting a device driver (these
>> patches will be released soon too) for an SPI device to the current kernel.
>
> There appeared to be a number of outstanding review comments (misleading
> commit message on patch 7, some concerns about the non-CMU case), please
> address those.

We discussed with Tomasz Figa and Krzysztof Kozłowski off the list that
this is practically unused. Tomasz, Krzysztof, would you be so kind to
share the details?

Kind regards,
Krzysztof Kozlowski Oct. 1, 2020, 7:02 p.m. UTC | #3
On Thu, Oct 01, 2020 at 08:23:00PM +0200, Lukasz Stelmach wrote:
> It was <2020-10-01 czw 17:13>, when Mark Brown wrote:
> > On Thu, Oct 01, 2020 at 05:21:39PM +0200, Łukasz Stelmach wrote:
> >> This is a series of fixes created during porting a device driver (these
> >> patches will be released soon too) for an SPI device to the current kernel.
> >
> > There appeared to be a number of outstanding review comments (misleading
> > commit message on patch 7, some concerns about the non-CMU case), please
> > address those.
> 
> We discussed with Tomasz Figa and Krzysztof Kozłowski off the list that
> this is practically unused. Tomasz, Krzysztof, would you be so kind to
> share the details?

That is correct. We did not provide final comments on the list so they
could be added here - in change log. This would also be an explanation
why there is a resend. Another solution would be to extend the commit #7
description - why only CMU case is covered.

About patch #7: The decision was not to correct non-CMU case because
there were not actual reports about clock rounding poblems, it would not
be trivial change and we do not have the HW to test.

Thanks Mark for looking into it.

Best regards,
Krzysztof
Mark Brown Oct. 1, 2020, 7:43 p.m. UTC | #4
On Thu, Oct 01, 2020 at 09:02:57PM +0200, Krzysztof Kozlowski wrote:

> That is correct. We did not provide final comments on the list so they
> could be added here - in change log. This would also be an explanation
> why there is a resend. Another solution would be to extend the commit #7
> description - why only CMU case is covered.

If there's a new changelog then it's not a resend, the changelog is part
of the content so I'd expect a version bump for that alone.  IIRC the
changelog needed some clarification anyway?
Krzysztof Kozlowski Oct. 2, 2020, 6:18 a.m. UTC | #5
On Thu, 1 Oct 2020 at 21:44, Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, Oct 01, 2020 at 09:02:57PM +0200, Krzysztof Kozlowski wrote:
>
> > That is correct. We did not provide final comments on the list so they
> > could be added here - in change log. This would also be an explanation
> > why there is a resend. Another solution would be to extend the commit #7
> > description - why only CMU case is covered.
>
> If there's a new changelog then it's not a resend, the changelog is part
> of the content so I'd expect a version bump for that alone.  IIRC the
> changelog needed some clarification anyway?

Yes, documenting the non-CMU case in changeloge would be good. It
should be also v3 because of another reason: two patches got reordered
to a more meaningful position in patchset, which brought minor
differences in them.

Best regards,
Krzysztof