| Message ID | 20240206085238.1208256-1-tudor.ambarus@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | spi: s3c64xx: add support for google,gs101-spi | expand |
On Tue, Feb 06, 2024 at 10:12:30AM +0000, Peter Griffin wrote: > The patch ordering seems a bit off with this series..I believe it should be > 1) dt-bindings patch (docs first) > 2) Add the use_32bit_io flag / functionality > 3) gs101 support (this patch) that uses the use_32bit_io functionality That's the ordering the series has? There's a random cleanup patch tacked on the front but that really ought to be separate anyway.
On 2/6/24 11:04, Mark Brown wrote: > On Tue, Feb 06, 2024 at 10:12:30AM +0000, Peter Griffin wrote: > >> The patch ordering seems a bit off with this series..I believe it should be >> 1) dt-bindings patch (docs first) >> 2) Add the use_32bit_io flag / functionality >> 3) gs101 support (this patch) that uses the use_32bit_io functionality > > That's the ordering the series has? There's a random cleanup patch > tacked on the front but that really ought to be separate anyway. I put the include <linux/types.h> patch first because I considered it a fix (driver is using u32) and because I need types.h in patch 3/4. Fixes first, then bindings, then driver. Was I wrong?
On Tue, 6 Feb 2024 at 11:19, Tudor Ambarus <tudor.ambarus@linaro.org> wrote: > > > > On 2/6/24 11:04, Mark Brown wrote: > > On Tue, Feb 06, 2024 at 10:12:30AM +0000, Peter Griffin wrote: > > > >> The patch ordering seems a bit off with this series..I believe it should be > >> 1) dt-bindings patch (docs first) > >> 2) Add the use_32bit_io flag / functionality > >> 3) gs101 support (this patch) that uses the use_32bit_io functionality > > > > That's the ordering the series has? There's a random cleanup patch > > tacked on the front but that really ought to be separate anyway. > > I put the include <linux/types.h> patch first because I considered it a > fix (driver is using u32) and because I need types.h in patch 3/4. Fixes > first, then bindings, then driver. > > Was I wrong? No my mistake, sorry for the noise. Gmail showed this driver change as the first patch after the cover letter but the subject was hidden so it wasn't obvious it was [4/4]
On Tue, Feb 6, 2024 at 2:52 AM Tudor Ambarus <tudor.ambarus@linaro.org> wrote: > > Depends on the simple cleanup patches from: > https://lore.kernel.org/linux-spi/20240205124513.447875-1-tudor.ambarus@linaro.org/ > > A slightly different version of the google,gs101-spi support was sent at: > https://lore.kernel.org/linux-spi/20240125145007.748295-1-tudor.ambarus@linaro.org/ > > Let's add support for gs101-spi so that I have a testing base for the > driver rework patches that will follow. > > Tudor Ambarus (4): > spi: s3c64xx: explicitly include <linux/types.h> > spi: dt-bindings: samsung: add google,gs101-spi compatible > spi: s3c64xx: add s3c64xx_iowrite{8,16}_32_rep accessors > spi: s3c64xx: add support for google,gs101-spi > Just a grumpy note: I wish this series (except for the [PATCH 1/4], which I'd argue doesn't belong here) was submitted before the rest of SPI cleanups and reworkings. Would've made reviewing much easier, because this series doesn't apply without SPI cleanup series that has to be applied prior to that. There are other benefits to that approach too, as was discussed earlier. > .../devicetree/bindings/spi/samsung,spi.yaml | 1 + > drivers/spi/spi-s3c64xx.c | 89 +++++++++++++++---- > 2 files changed, 75 insertions(+), 15 deletions(-) > > -- > 2.43.0.594.gd9cf4e227d-goog >
On 2/6/24 18:59, Sam Protsenko wrote: > On Tue, Feb 6, 2024 at 2:52 AM Tudor Ambarus <tudor.ambarus@linaro.org> wrote: >> >> Depends on the simple cleanup patches from: >> https://lore.kernel.org/linux-spi/20240205124513.447875-1-tudor.ambarus@linaro.org/ >> >> A slightly different version of the google,gs101-spi support was sent at: >> https://lore.kernel.org/linux-spi/20240125145007.748295-1-tudor.ambarus@linaro.org/ >> >> Let's add support for gs101-spi so that I have a testing base for the >> driver rework patches that will follow. >> >> Tudor Ambarus (4): >> spi: s3c64xx: explicitly include <linux/types.h> >> spi: dt-bindings: samsung: add google,gs101-spi compatible >> spi: s3c64xx: add s3c64xx_iowrite{8,16}_32_rep accessors >> spi: s3c64xx: add support for google,gs101-spi >> > > Just a grumpy note: I wish this series (except for the [PATCH 1/4], > which I'd argue doesn't belong here) was submitted before the rest of > SPI cleanups and reworkings. Would've made reviewing much easier, > because this series doesn't apply without SPI cleanup series that has > to be applied prior to that. There are other benefits to that approach > too, as was discussed earlier. > I feel we're bike-shedding, it drains my energy. Your reasons were: 1/ easier review 2/ easier backporting of gs101 if that's ever wanted 3/ driver rework takes more review time and I risk not having gs101 integrated for next release 2/ is not true right now, I could cherry-pick the iowrite and gs101 patches on top of v6.7. With 1/ I don't agree as the gs101 patches are the same with or without the simple cleanup. 3/ is my responsibility and I'm ok with it, I feel there's enough time for all What matters, as I specified in the cover letter, is to have the gs101 patches before the functional driver rework which will follow, so that I can test each functional patch with gs101. I give up however, I'll do as you want. Will respin all.
Depends on the simple cleanup patches from: https://lore.kernel.org/linux-spi/20240205124513.447875-1-tudor.ambarus@linaro.org/ A slightly different version of the google,gs101-spi support was sent at: https://lore.kernel.org/linux-spi/20240125145007.748295-1-tudor.ambarus@linaro.org/ Let's add support for gs101-spi so that I have a testing base for the driver rework patches that will follow. Tudor Ambarus (4): spi: s3c64xx: explicitly include <linux/types.h> spi: dt-bindings: samsung: add google,gs101-spi compatible spi: s3c64xx: add s3c64xx_iowrite{8,16}_32_rep accessors spi: s3c64xx: add support for google,gs101-spi .../devicetree/bindings/spi/samsung,spi.yaml | 1 + drivers/spi/spi-s3c64xx.c | 89 +++++++++++++++---- 2 files changed, 75 insertions(+), 15 deletions(-)