Message ID | mafs0bk0tf9na.fsf@kernel.org |
---|---|
State | Not Applicable |
Headers | show |
Series | [GIT,PULL] mtd: spi-nor: changes for v6.12 | expand |
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. Thanks! Miquèl
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 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.
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