| Message ID | 20240410-cdns-qspi-mbly-v3-0-7b7053449cf7@bootlin.com |
|---|---|
| Headers | show |
| Series | spi: cadence-qspi: add Mobileye EyeQ5 support | expand |
On Wed, Apr 10, 2024 at 11:29:03AM +0200, Théo Lebrun wrote:
> V1 cover letter [5] contains a brief summary of what gets added.
Please make your cover letters stand alone things, things like a basic
description of the contents of the series should just be included
directly.
On Wed, Apr 10, 2024 at 11:29:07AM +0200, Théo Lebrun wrote: > If FIFO depth DT property is provided, check it matches what hardware > reports and warn otherwise. Else, use hardware provided value. > > Hardware exposes FIFO depth indirectly because > CQSPI_REG_SRAMPARTITION is partially read-only. This breaks an allmodconfig build: /build/stage/linux/drivers/spi/spi-cadence-quadspi.c: In function ‘cqspi_of_get_ pdata’: /build/stage/linux/drivers/spi/spi-cadence-quadspi.c:1506:45: error: unused vari able ‘ddata’ [-Werror=unused-variable] 1506 | const struct cqspi_driver_platdata *ddata = cqspi->ddata; | ^~~~~ /build/stage/linux/drivers/spi/spi-cadence-quadspi.c: In function ‘cqspi_control ler_detect_fifo_depth’: /build/stage/linux/drivers/spi/spi-cadence-quadspi.c:1582:45: error: unused vari able ‘ddata’ [-Werror=unused-variable] 1582 | const struct cqspi_driver_platdata *ddata = cqspi->ddata; | ^~~~~ cc1: all warnings being treated as errors
Hello, On Wed Apr 10, 2024 at 10:03 PM CEST, Mark Brown wrote: > On Wed, Apr 10, 2024 at 11:29:07AM +0200, Théo Lebrun wrote: > > > If FIFO depth DT property is provided, check it matches what hardware > > reports and warn otherwise. Else, use hardware provided value. > > > > Hardware exposes FIFO depth indirectly because > > CQSPI_REG_SRAMPARTITION is partially read-only. > > This breaks an allmodconfig build: > > /build/stage/linux/drivers/spi/spi-cadence-quadspi.c: In function ‘cqspi_of_get_ > pdata’: > /build/stage/linux/drivers/spi/spi-cadence-quadspi.c:1506:45: error: unused vari > able ‘ddata’ [-Werror=unused-variable] > 1506 | const struct cqspi_driver_platdata *ddata = cqspi->ddata; > | ^~~~~ > /build/stage/linux/drivers/spi/spi-cadence-quadspi.c: In function ‘cqspi_control > ler_detect_fifo_depth’: > /build/stage/linux/drivers/spi/spi-cadence-quadspi.c:1582:45: error: unused vari > able ‘ddata’ [-Werror=unused-variable] > 1582 | const struct cqspi_driver_platdata *ddata = cqspi->ddata; > | ^~~~~ > cc1: all warnings being treated as errors I really should fix my kernel compiler warnings. Sorry about that. Will fix next revision. Regards, -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Wed, 10 Apr 2024 11:29:03 +0200, Théo Lebrun wrote: > V3 of this series adding octal SPI-NOR support to Mobileye EyeQ5 > platform. It has been tested on EyeQ5 hardware successfully. > V1 cover letter [5] contains a brief summary of what gets added. > > There is no dependency except if you want zero errors in devicetree: > system-controller series [3] for <&clocks> phandle. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next Thanks! [1/9] spi: dt-bindings: cdns,qspi-nor: sort compatibles alphabetically commit: 002514d91fccde2adbe750c9ec5c6207d56c890b [2/9] spi: dt-bindings: cdns,qspi-nor: add mobileye,eyeq5-ospi compatible commit: 52826aee484b3ebb6ed94c1ae89c0944110ed8b1 [3/9] spi: dt-bindings: cdns,qspi-nor: make cdns,fifo-depth optional commit: eb4fdb4bf46f875eac3c093f7ff43a223985f7b8 [4/9] spi: cadence-qspi: allow FIFO depth detection (no commit info) [5/9] spi: cadence-qspi: add no-IRQ mode to indirect reads (no commit info) [6/9] spi: cadence-qspi: add early busywait to cqspi_wait_for_bit() (no commit info) [7/9] spi: cadence-qspi: add mobileye,eyeq5-ospi compatible (no commit info) All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
Hello Mark, On Thu Apr 11, 2024 at 2:03 PM CEST, Mark Brown wrote: > On Wed, 10 Apr 2024 11:29:03 +0200, Théo Lebrun wrote: > > V3 of this series adding octal SPI-NOR support to Mobileye EyeQ5 > > platform. It has been tested on EyeQ5 hardware successfully. > > V1 cover letter [5] contains a brief summary of what gets added. > > > > There is no dependency except if you want zero errors in devicetree: > > system-controller series [3] for <&clocks> phandle. > > > > [...] > > Applied to > > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next > > Thanks! > > [1/9] spi: dt-bindings: cdns,qspi-nor: sort compatibles alphabetically > commit: 002514d91fccde2adbe750c9ec5c6207d56c890b > [2/9] spi: dt-bindings: cdns,qspi-nor: add mobileye,eyeq5-ospi compatible > commit: 52826aee484b3ebb6ed94c1ae89c0944110ed8b1 > [3/9] spi: dt-bindings: cdns,qspi-nor: make cdns,fifo-depth optional > commit: eb4fdb4bf46f875eac3c093f7ff43a223985f7b8 > [4/9] spi: cadence-qspi: allow FIFO depth detection > (no commit info) > [5/9] spi: cadence-qspi: add no-IRQ mode to indirect reads > (no commit info) > [6/9] spi: cadence-qspi: add early busywait to cqspi_wait_for_bit() > (no commit info) > [7/9] spi: cadence-qspi: add mobileye,eyeq5-ospi compatible > (no commit info) All commits tagged "(no commit info)" do not show up in your for-next branch. Is that expected and is there anything I can do? There was one pending -Wunused-variable compiler warning to be addressed for example, see [0]. ⟩ git log --oneline --author theo.lebrun v6.9-rc1..spi/for-next eb4fdb4bf46f spi: dt-bindings: cdns,qspi-nor: make cdns,fifo-depth optional 52826aee484b spi: dt-bindings: cdns,qspi-nor: add mobileye,eyeq5-ospi compatible 002514d91fcc spi: dt-bindings: cdns,qspi-nor: sort compatibles alphabetically 563f8598cbc2 spi: cadence-qspi: minimise register accesses on each op if !DTR dcc594aef1bf spi: cadence-qspi: store device data pointer in private struct 708eafeba9ee spi: cadence-qspi: allow building for MIPS [0]: https://lore.kernel.org/lkml/161eebc1-9417-4ab0-ad8c-c1b17be119b4@sirena.org.uk/ Thanks, -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Mon, Apr 22, 2024 at 06:52:47PM +0200, Théo Lebrun wrote: > All commits tagged "(no commit info)" do not show up in your for-next > branch. Is that expected and is there anything I can do? There was one > pending -Wunused-variable compiler warning to be addressed for > example, see [0]. Please submit any patches you'd like to see included. If there were outstanding issues that need fixing then fixing those prior to submitting would be sensible.
Hello, On Tue Apr 23, 2024 at 7:00 AM CEST, Mark Brown wrote: > On Mon, Apr 22, 2024 at 06:52:47PM +0200, Théo Lebrun wrote: > > All commits tagged "(no commit info)" do not show up in your for-next > > branch. Is that expected and is there anything I can do? There was one > > pending -Wunused-variable compiler warning to be addressed for > > example, see [0]. > > Please submit any patches you'd like to see included. If there were > outstanding issues that need fixing then fixing those prior to > submitting would be sensible. Seeing "Applied" followed by a list of commits, with some of those not being applied confused me. You received the latest revision! https://lore.kernel.org/lkml/20240423-cdns-qspi-mbly-v4-0-3d2a7b535ad0@bootlin.com/ Thanks, -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On 23/04/2024 12:04, Théo Lebrun wrote: > Hello, > > On Tue Apr 23, 2024 at 7:00 AM CEST, Mark Brown wrote: >> On Mon, Apr 22, 2024 at 06:52:47PM +0200, Théo Lebrun wrote: >>> All commits tagged "(no commit info)" do not show up in your for-next >>> branch. Is that expected and is there anything I can do? There was one >>> pending -Wunused-variable compiler warning to be addressed for >>> example, see [0]. >> >> Please submit any patches you'd like to see included. If there were >> outstanding issues that need fixing then fixing those prior to >> submitting would be sensible. > > Seeing "Applied" followed by a list of commits, with some of those not > being applied confused me. That's a standard output of b4 and maybe also Patchwork, if some parts are applied. Best regards, Krzysztof
Hello, On Tue Apr 23, 2024 at 12:25 PM CEST, Krzysztof Kozlowski wrote: > On 23/04/2024 12:04, Théo Lebrun wrote: > > Hello, > > > > On Tue Apr 23, 2024 at 7:00 AM CEST, Mark Brown wrote: > >> On Mon, Apr 22, 2024 at 06:52:47PM +0200, Théo Lebrun wrote: > >>> All commits tagged "(no commit info)" do not show up in your for-next > >>> branch. Is that expected and is there anything I can do? There was one > >>> pending -Wunused-variable compiler warning to be addressed for > >>> example, see [0]. > >> > >> Please submit any patches you'd like to see included. If there were > >> outstanding issues that need fixing then fixing those prior to > >> submitting would be sensible. > > > > Seeing "Applied" followed by a list of commits, with some of those not > > being applied confused me. > > That's a standard output of b4 and maybe also Patchwork, if some parts > are applied. Thanks for the pointer. I've created an issue over at b4 to see what people think about this matter. Current behavior is not intuitive as a young contributor. See: https://github.com/mricon/b4/issues/26 Regards, -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Tue, Apr 23, 2024 at 03:08:05PM +0200, Théo Lebrun wrote: > Hello, > > On Tue Apr 23, 2024 at 12:25 PM CEST, Krzysztof Kozlowski wrote: > > On 23/04/2024 12:04, Théo Lebrun wrote: > > > Hello, > > > > > > On Tue Apr 23, 2024 at 7:00 AM CEST, Mark Brown wrote: > > >> On Mon, Apr 22, 2024 at 06:52:47PM +0200, Théo Lebrun wrote: > > >>> All commits tagged "(no commit info)" do not show up in your for-next > > >>> branch. Is that expected and is there anything I can do? There was one > > >>> pending -Wunused-variable compiler warning to be addressed for > > >>> example, see [0]. > > >> > > >> Please submit any patches you'd like to see included. If there were > > >> outstanding issues that need fixing then fixing those prior to > > >> submitting would be sensible. > > > > > > Seeing "Applied" followed by a list of commits, with some of those not > > > being applied confused me. > > > > That's a standard output of b4 and maybe also Patchwork, if some parts > > are applied. > > Thanks for the pointer. I've created an issue over at b4 to see what > people think about this matter. Current behavior is not intuitive as a > young contributor. FWIW, given I see `having a more confident comment such as "(commit not applied)".` there, having (no commit info) doesn't mean that it wasn't applied always. Sometimes I've found that due to changes in the patch b4 could not detect that it was applied and reported (no commit info).
On Tue, Apr 23, 2024 at 06:23:16PM +0100, Conor Dooley wrote: > On Tue, Apr 23, 2024 at 03:08:05PM +0200, Théo Lebrun wrote: > > Thanks for the pointer. I've created an issue over at b4 to see what > > people think about this matter. Current behavior is not intuitive as a > > young contributor. > FWIW, given I see `having a more confident comment such as > "(commit not applied)".` there, having (no commit info) doesn't mean > that it wasn't applied always. Sometimes I've found that due to changes > in the patch b4 could not detect that it was applied and reported (no > commit info). Right, it can't prove a negative - if it can't find the patch it could be because it wasn't sent against current code and got changed sufficiently in application to cause issues.
On Wed, Apr 24, 2024 at 10:01:56AM +0900, Mark Brown wrote: > > > Thanks for the pointer. I've created an issue over at b4 to see what > > > people think about this matter. Current behavior is not intuitive as a > > > young contributor. > > > FWIW, given I see `having a more confident comment such as > > "(commit not applied)".` there, having (no commit info) doesn't mean > > that it wasn't applied always. Sometimes I've found that due to changes > > in the patch b4 could not detect that it was applied and reported (no > > commit info). > > Right, it can't prove a negative - if it can't find the patch it could > be because it wasn't sent against current code and got changed > sufficiently in application to cause issues. We can also be a bit more relaxed. For example, we can look at consecutive commits and compare the subjects to see if there's a match. I'll see if that's something I can add in. In general, though, I prefer to push people in a different direction -- we really shouldn't be fixing up people's patches, because this misattributes the code to the wrong author. Instead, we really should either ask senders to send an updated revision, or make the changes in merge commits instead. Merge commits can be created using "b4 shazam -M", but I must admit that editing the contents of the merge commit isn't really that straightforward, unfortunately. -K
On Wed, Apr 24, 2024 at 03:53:03PM -0400, Konstantin Ryabitsev wrote: > In general, though, I prefer to push people in a different direction -- > we really shouldn't be fixing up people's patches, because this > misattributes the code to the wrong author. Instead, we really should > either ask senders to send an updated revision, or make the changes in > merge commits instead. I don't do that - if this triggers with any of my stuff it's either that the patch was dropped or git am did something.
Hi all, V3 of this series adding octal SPI-NOR support to Mobileye EyeQ5 platform. It has been tested on EyeQ5 hardware successfully. V1 cover letter [5] contains a brief summary of what gets added. There is no dependency except if you want zero errors in devicetree: system-controller series [3] for <&clocks> phandle. Have a nice day, Théo [0]: https://lore.kernel.org/lkml/20240216174227.409400-1-gregory.clement@bootlin.com/ [1]: https://lore.kernel.org/linux-mips/20240209-regname-v1-0-2125efa016ef@flygoat.com/ [2]: https://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git/log/ [3]: https://lore.kernel.org/lkml/20240301-mbly-clk-v9-0-cbf06eb88708@bootlin.com/ [4]: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git/log/ [5]: https://lore.kernel.org/lkml/20240308-cdns-qspi-mbly-v1-0-a503856dd205@bootlin.com/ [6]: https://lore.kernel.org/lkml/171259906078.120310.15397790336440498713.b4-ty@kernel.org/ Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> --- Changes in v3: - dt-bindings: - Patch "sort compatibles alphabetically": - Moved first. - Take Reviewed-By Krzysztof. - Patch "add mobileye,eyeq5-ospi compatible": - EyeQ5 no longer implies no cdns,fifo-depth prop. Patch now only adds compatible, no more property conditional. - New "make cdns,fifo-depth optional" patch, for all compatibles. - Driver: - FIFO depth detection is no longer a quirk. It is for all compatibles if no DT property is provided. - Rebase onto spi-next [4] to drop three patches. No-IRQ mode patch is mentioned in email saying a subset of patches got applied [6]. However, it is not in spi-next, so it is kept in series. - Busywait is no longer behind a quirk; it applies to all compatibles. - No-IRQ mode patch got modified to change its quirk index because previous quirk got removed. - As we removed some quirks, we no longer overflow u8 quirks. - Link to v2: https://lore.kernel.org/r/20240405-cdns-qspi-mbly-v2-0-956679866d6d@bootlin.com Changes in v2: - Rebase upon v6.9-rc2. - Fix dt-bindings commit subject tags. - Take Reviewed-by: Krzysztof Kozlowski on dt-bindings commit. - Add dt-bindings commit to order compatibles alphabetically. Krzysztof: unsure if you want this. It is second so that commit adding EyeQ5 compatible can be taken alone easily. - Drop patch taken upstream: spi: cadence-qspi: switch from legacy names to modern ones - Add To: Rob Herring, following get_maintainer.pl recommendation. - Link to v1: https://lore.kernel.org/r/20240308-cdns-qspi-mbly-v1-0-a503856dd205@bootlin.com --- Théo Lebrun (9): spi: dt-bindings: cdns,qspi-nor: sort compatibles alphabetically spi: dt-bindings: cdns,qspi-nor: add mobileye,eyeq5-ospi compatible spi: dt-bindings: cdns,qspi-nor: make cdns,fifo-depth optional spi: cadence-qspi: allow FIFO depth detection spi: cadence-qspi: add no-IRQ mode to indirect reads spi: cadence-qspi: add early busywait to cqspi_wait_for_bit() spi: cadence-qspi: add mobileye,eyeq5-ospi compatible MIPS: mobileye: eyeq5: Add SPI-NOR controller node MIPS: mobileye: eyeq5: add octal flash node to eval board DTS .../devicetree/bindings/spi/cdns,qspi-nor.yaml | 8 +- arch/mips/boot/dts/mobileye/eyeq5-epm5.dts | 15 ++++ arch/mips/boot/dts/mobileye/eyeq5.dtsi | 15 ++++ drivers/spi/spi-cadence-quadspi.c | 93 +++++++++++++++++----- 4 files changed, 108 insertions(+), 23 deletions(-) --- base-commit: d442072c067c86787dcee22c5d30d36b14edbba7 change-id: 20240209-cdns-qspi-mbly-de2205a44ab3 Best regards,