Re: [PATCH] Revert "spi: modify set_cs_timing parameter"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 30.09.21 14:46, Mark Brown wrote:
On Thu, Sep 30, 2021 at 02:36:01PM +0200, Dafna Hirschfeld wrote:
hi, thanks for the fast feedback

On 30.09.21 14:25, Mark Brown wrote:
On Thu, Sep 30, 2021 at 02:07:00PM +0200, Dafna Hirschfeld wrote:
This reverts commit 04e6bb0d6bb127bac929fb35edd2dd01613c9520.

Which is not what the commit message nor the paste of the full hash
claimed :/

What is the paste of the full hash?

The above.

Since the second commit is only a warning fixes I thought it is cumbersome to
send two separate reverting patches. Should I?

No, you should write a proper commit log with (like I said) a normal
subject line - basically, follow the process in submitting-patches.rst.

Do we have any analysis as to why?  Do these devices use timing
parameters in some way for example, or do the values written out to the
device change in some way?

You've provided no analysis here so it's hard to tell if this is just
some random change that happens to change code generation slighly or if
there's some actual reason why this might fix something.  I'll note that
as far as I can see there are no users of this API upstream so I'm
guessing that you've got some out of tree consumer driver which uses the
API, it's possible that there was some error in updating that driver to
the new interface which is causing the issue.

Actually the original commit not only change that callback 'set_cs_timing' but it also
calls 'mtk_spi_set_hw_cs_timing' directly from the function "mtk_spi_prepare_message".
So this actually influences all devices bound to this driver (in upstream)
I did some printing and it does change values that are written to registers.

OK, so that's something that should have been in the commit log,
preferrably in a more detailed form that identifies what the change is.
However changing the values written out is clearly not the intent of the
patch and it is a substantially better API so can we not just fix things
so that the old values are written out?  Why are we jumping straight to
a revert here?

It could be that the values written to the register in the new version of "mtk_spi_set_hw_cs_timing" are the same
as with the previous version. I didn't check that. The difference is that before that patch
the function was not called so it was a dead code. Now it is called and causes erros.
Without the datasheet it is hard to know how to fix it. I responded to that patch two days ago explaining
that but Mason Zhang didn't respond yet.

Thanks,
Dafna





[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux