Re: [GIT PULL] mtd: spi-nor: changes for v6.12

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Pratyush,

pratyush@kernel.org wrote on Sat, 14 Sep 2024 17:55:39 +0200:

> On Fri, Sep 13 2024, Miquel Raynal wrote:
> 
> > Hi Pratyush,
> >
> > pratyush@kernel.org wrote on Thu, 12 Sep 2024 12:28:57 +0200:
> >  
> >> Hi Miquel,
> >> 
> >> Here are the SPI NOR changes for v6.12. I usually base my branch on top
> >> of -rc1, but this time around it seems I did it a few commits after
> >> v6.11-rc1. Probably just didn't notice torvalds/master had moved. Should
> >> make no difference in practice anyway.  
> >
> > I don't think I can merge your tag without the 4 minmax patches, which
> > means they will appear in my final merge request to Linus, unless I
> > explicitly don't use an -rc as a base, but this must be justified I
> > believe. Can you please fix the branch and regenerate the tag? I don't
> > mind personally if you force push, if it makes the history more clear.  
> 
> TL;DR: I'll rebase and send you a new pull request.
> 
> I thought it wouldn't matter since Linus' tree already has those minmax
> commits anyway. I did a test merge just now and seems I was right. When
> I merge current spi-nor/next into mtd/next, and then merge mtd/next into
> torvalds/master, here's the merge I get:
> 
>     Merge branch 'mtd/merge-test'
> 
>     * mtd/merge-test:
>     mtd: spi-nor: fix flash probing
>     mtd: powernv: Add check devm_kasprintf() returned value
>     mtd: spi-nor: spansion: Add support for S28HS256T
>     mtd: concat: Use kmemdup_array instead of kmemdup for multiple allocation
>     mtd: parsers: bcm47xxpart: make read-only array possible_nvram_sizes static const
>     mtd: Use of_property_read_bool()
>     mtd: slram: insert break after errors in parsing the map
>     mtd: spi-nor: winbond: add Zetta ZD25Q128C support
>     mtd: spi-nor: micron-st: Add n25q064a WP support
>     mtd: spi-nor: sst: Factor out common write operation to `sst_nor_write_data()`
> 
> But since mtd/next doesn't have the minmax commits, here is what the
> merge of spi-nor/next into mtd/next looks like:
> 
>     Merge branch 'spi-nor/next' into mtd/merge-test
> 
>     * spi-nor/next:
>     mtd: spi-nor: fix flash probing
>     mtd: spi-nor: spansion: Add support for S28HS256T
>     mtd: spi-nor: winbond: add Zetta ZD25Q128C support
>     mtd: spi-nor: micron-st: Add n25q064a WP support
>     mtd: spi-nor: sst: Factor out common write operation to `sst_nor_write_data()`
>     minmax: simplify min()/max()/clamp() implementation
>     minmax: don't use max() in situations that want a C constant expression
>     minmax: scsi: fix mis-use of 'clamp()' in sr.c
>     minmax: make generic MIN() and MAX() macros available everywhere

Yes, this one looks bad, but you're right the final merge request would
have been clean.

> Essentially, your merge to Linus would be fine but my merge to your
> branch will (appear to) have these extra commits.
> 
> I don't think any of this is worth the extra confusion so I will just
> rebase my branch on v6.11-rc1 force push. Will send you a v2 of the pull
> request soon.
> 
> Side note:
> 
> > [...]unless I explicitly don't use an -rc as a base, but this must be
> > justified I believe.  
> 
> I am curious why that is so. I don't see how using an -rc as base is any
> better than using any other commit in Linus' tree. For git it doesn't
> matter since an -rc commit is the same as any other commit. I suppose if
> everyone does it, the history might be a bit cleaner, but I don't see
> how it would make much of a difference in practice.

Yes, I guess for cleanliness purposes it is expected that people base
their branches on -rc tags, so it is easier to catch what is part of
their work? Otherwise if you _need_ another patch in-between I believe
it should be stated why you need it.

Anyway, thanks for the v2!

Cheers,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/