Re: [PATCH v2 00/13] nvmem: patches (set 1) for 6.15

[Srinivas Kandagatla <srinivas.kandagatla@linaro.org>]

On 03/04/2025 14:52, Greg KH wrote:
> On Thu, Apr 03, 2025 at 12:38:03PM +0300, Dmitry Baryshkov wrote:
>> On 03/04/2025 12:35, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 03/04/2025 10:31, Dmitry Baryshkov wrote:
>>>> On Thu, 3 Apr 2025 at 12:27, Srinivas Kandagatla
>>>> <srinivas.kandagatla@linaro.org> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 03/04/2025 10:25, Dmitry Baryshkov wrote:
>>>>>> On 03/04/2025 12:18, Srinivas Kandagatla wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 02/04/2025 12:31, Greg KH wrote:
>>>>>>>> On Wed, Apr 02, 2025 at 09:19:17AM +0100, Srinivas Kandagatla wrote:
>>>>>>>>> HI Greg,
>>>>>>>>>
>>>>>>>>> On 01/04/2025 20:18, Greg KH wrote:
>>>>>>>>>> On Sun, Mar 09, 2025 at 02:56:50PM +0000,
>>>>>>>>>> srinivas.kandagatla@linaro.org wrote:
>>>>>>>>>>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>>>>>>>>>>
>>>>>>>>>>> Hi Greg,
>>>>>>>>>>>
>>>>>>>>>>> Here are few nvmem patches for 6.15, Could you queue
>>>>>>>>>>> these for 6.15.
>>>>>>>>>>>
>>>>>>>>>>> patche include
>>>>>>>>>>>       - updates to bindings to include MSM8960, X1E80100, MS8937,
>>>>>>>>>>>         IPQ5018
>>>>>>>>>>>       - add support to bit offsets for register strides exceeding
>>>>>>>>>>>         single byte
>>>>>>>>>>>       - add rockchip-otp variants.
>>>>>>>>>>>       - Few enhancements in qfprom and rochchip nvmem providers.
>>>>>>>>>>
>>>>>>>>>> Ok, I wanted to apply these, and tried to, but they fail horribly
>>>>>>>>>> because:
>>>>>>>>>>
>>>>>>>>>> Commit: 1b14625bd6d4 ("nvmem: qfprom: switch
>>>>>>>>>> to 4-byte aligned reads")
>>>>>>>>>>       Fixes tag: Fixes: 11ccaa312111 ("nvmem: core: verify cell's
>>>>>>>>>> raw_len")
>>>>>>>>>>       Has these problem(s):
>>>>>>>>>>           - Target SHA1 does not exist
>>>>>>>>>> Commit: a8a7c7e34093 ("nvmem: core: update raw_len if the bit
>>>>>>>>>> reading is required")
>>>>>>>>>>       Fixes tag: Fixes: 11ccaa312111 ("nvmem: core: verify cell's
>>>>>>>>>> raw_len")
>>>>>>>>>>       Has these problem(s):
>>>>>>>>>>           - Target SHA1 does not exist
>>>>>>>>>> Commit: d44f60348d8c ("nvmem: core: fix bit offsets of more than
>>>>>>>>>> one byte")
>>>>>>>>>>       Fixes tag: Fixes: 11ccaa312111 ("nvmem: core: verify cell's
>>>>>>>>>> raw_len")
>>>>>>>>>>       Has these problem(s):
>>>>>>>>>>           - Target SHA1 does not exist
>>>>>>>>>
>>>>>>>>> Looks some of your scripts or b4 is picking up
>>>>>>>>> older version v1 of the
>>>>>>>>> patchset
>>>>>>>>>
>>>>>>>>> None of the above patches have Fixes tags in the V2 patches that I
>>>>>>>>> shared
>>>>>>>>> aswell as patches in linux-next.
>>>>>>>>
>>>>>>>> Yes, that looks odd, it looks like b4 pulled in the
>>>>>>>> wrong series, yes.
>>>>>>>>
>>>>>>>
>>>>>>> Even that looked incorrect, as the v1 series only had one
>>>>>>> patch("[PATCH 12/14] nvmem: make the misaligned raw_len non-fatal")
>>>>>>> that had fixes tag. Not sure how these 3 patches are tagged as fixes.
>>>>>>>
>>>>>>>> But, that's even worse.  Those "fixes" are now not actually marked as
>>>>>>>> fixes of the previous patch.  So that information is
>>>>>>>> totally lost, and
>>>>>>>
>>>>>>> Its because this patch("PATCH 12/14] nvmem: make the misaligned
>>>>>>> raw_len non-fatal") is taken as fixup patch and wrapped into the
>>>>>>> original patch ("nvmem: core: verify cell's raw_len"), Also the sha
>>>>>>> will not be valid for linus or char-misc tree.
>>>>>>>
>>>>>>>> again, the first commit here, "nvmem: core: verify cell's raw_len" is
>>>>>>>> broken so much that it took 3 other changes to fix it, which implies
>>>>>>>> that bisection would cause problems if you hit it in the middle here.
>>>>>>>>
>>>>>>>
>>>>>>> All the patches related to this are enhancements to nvmem core to
>>>>>>> allow specifying bit offsets for nvmem cell that have 4 bytes strides.
>>>>>>>
>>>>>>> Specially this check is also an additional check in core to make sure
>>>>>>> that cell offsets are aligned to register strides.
>>>>>>>
>>>>>>>> While fixing patches is great, and something we do
>>>>>>>> in the tree all the
>>>>>>>> time, let's not purposefully break things and then fix them up in the
>>>>>>>> same exact patch series please.  That's just sloppy engineering.
>>>>>>>>
>>>>>>>> Please redo this series completely.  I can take the
>>>>>>>> "new device support"
>>>>>>>
>>>>>>> I can send them but its going to be exactly same series, I dont think
>>>>>>> anything will change as all of these patches are enhancements and
>>>>>>> there are no fixes.
>>>>>>>
>>>>>>> I hope this clarifies a bit, Please let me know if you still want me
>>>>>>> to resend this series, which is going to be exactly same.
>>>>>>
>>>>>> I think Greg is asking to squash the fixup into the relevant patch.
>>>>>
>>>>> Its already squashed up in v2.
>>>>
>>>> Then there should be no Fixes tags. Is the series that you are sending
>>>> visible somewhere?
>>>
>>> Yes, there is no fixes tags in v2 series,
>>>
>>> Here is what is sent to as v2:
>>> https://lore.kernel.org/lkml/47a3a851-
>>> f737-4772-87c6-98613347435c@linaro.org/T/
>>> #r01449e17cf6f9396967a822a0460ad4b1245e914
>>
>> LGTM, thanks. Then I don't understand what is causing the controversy from
>> Greg's side. The only piece of information that got lost is that Mark has
>> found an issue with the previous version of the patch (I'd have added that
>> information between the tags as you've squashed the patches).
> 
> Yeah, I'm confused here as well.  In v1, there were 3 patches that were
> marked as "Fixes" for a previous patch in the series.  In v2, no Fixes
> were marked at all, BUT the patches were still in the series.

In fact there was only one patch with fixes tag in v1, not sure if b4 
picked up 3 which is the part that confused me too.

> 
> So what went wrong?  Was the v2 version of the original patch changed so
> that the 3 other ones were not needed somehow?  If so, why were they in
> the list again?

I have captured that in cover letter:


Changes since v1:
  - Merged fixup "nvmem: make the misaligned raw_len non-fatal" into
   "nvmem: core: verify cell's raw_len"


> 
> Anyway, I'm confused...
> 
> Please send a v3 of this series, NOT in response to any email thread so
> that b4 does NOT get confused in any way, and I'll be glad to review
> them and apply them to the proper branch after -rc1 is out next week or
> so.

I will send v3.
thanks,
Srini
> 
> And document the heck out of what has changed in the series in the
> different patches please.
> 

> thanks,
> 
> greg k-h