Documentation/spi/pxa2xx.rst | 208 --------------------------------- arch/arm/mach-pxa/spitz.c | 25 ++-- drivers/spi/Kconfig | 5 +- drivers/spi/spi-pxa2xx-dma.c | 11 +- drivers/spi/spi-pxa2xx-pci.c | 10 +- drivers/spi/spi-pxa2xx.c | 99 ++++++++++------ drivers/spi/spi-pxa2xx.h | 39 ++++++- include/linux/spi/pxa2xx_spi.h | 48 -------- 8 files changed, 133 insertions(+), 312 deletions(-) delete mode 100644 Documentation/spi/pxa2xx.rst delete mode 100644 include/linux/spi/pxa2xx_spi.h
As Arnd suggested we may drop linux/spi/pxa2xx_spi.h as most of its content is being used solely internally to SPI subsystem (PXA2xx drivers). Hence this refactoring series with the additional win of getting rid of legacy documentation. Changelog v2: - dropped applied patches - added patch to amend dependencies (Mark) - amended the second patch accordingly (Mark) - elaborated purpose of the patch 6 in the commit message (Mark) Cc: Arnd Bergmann <arnd@arndb.de> Andy Shevchenko (9): spi: pxa2xx: Narrow the Kconfig option visibility spi: pxa2xx: Drop ACPI_PTR() and of_match_ptr() spi: pxa2xx: Extract pxa2xx_spi_init_ssp() helper spi: pxa2xx: Skip SSP initialization if it's done elsewhere spi: pxa2xx: Allow number of chip select pins to be read from property spi: pxa2xx: Provide num-cs for Sharp PDAs via device properties spi: pxa2xx: Move contents of linux/spi/pxa2xx_spi.h to a local one spi: pxa2xx: Remove outdated documentation spi: pxa2xx: Don't use "proxy" headers Documentation/spi/pxa2xx.rst | 208 --------------------------------- arch/arm/mach-pxa/spitz.c | 25 ++-- drivers/spi/Kconfig | 5 +- drivers/spi/spi-pxa2xx-dma.c | 11 +- drivers/spi/spi-pxa2xx-pci.c | 10 +- drivers/spi/spi-pxa2xx.c | 99 ++++++++++------ drivers/spi/spi-pxa2xx.h | 39 ++++++- include/linux/spi/pxa2xx_spi.h | 48 -------- 8 files changed, 133 insertions(+), 312 deletions(-) delete mode 100644 Documentation/spi/pxa2xx.rst delete mode 100644 include/linux/spi/pxa2xx_spi.h -- 2.43.0.rc1.1.gbec44491f096
On Wed, 27 Mar 2024 21:29:19 +0200, Andy Shevchenko wrote:
> As Arnd suggested we may drop linux/spi/pxa2xx_spi.h as most of
> its content is being used solely internally to SPI subsystem
> (PXA2xx drivers). Hence this refactoring series with the additional
> win of getting rid of legacy documentation.
>
> Changelog v2:
> - dropped applied patches
> - added patch to amend dependencies (Mark)
> - amended the second patch accordingly (Mark)
> - elaborated purpose of the patch 6 in the commit message (Mark)
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
Thanks!
[1/9] spi: pxa2xx: Narrow the Kconfig option visibility
commit: 3af201a405b3e5abee65102b062c309fff68cc0e
[2/9] spi: pxa2xx: Drop ACPI_PTR() and of_match_ptr()
commit: 9907c475dcab9b269422972577360122129ac84c
[3/9] spi: pxa2xx: Extract pxa2xx_spi_init_ssp() helper
commit: 7290f1e4075d28ab961df5a454503296fa289271
[4/9] spi: pxa2xx: Skip SSP initialization if it's done elsewhere
commit: bb77c99ee6d3d704086acf141d3ec92601747809
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
On Fri, Mar 29, 2024 at 01:29:10AM +0000, Mark Brown wrote: > On Wed, 27 Mar 2024 21:29:19 +0200, Andy Shevchenko wrote: > > As Arnd suggested we may drop linux/spi/pxa2xx_spi.h as most of > > its content is being used solely internally to SPI subsystem > > (PXA2xx drivers). Hence this refactoring series with the additional > > win of getting rid of legacy documentation. > > > > Changelog v2: > > - dropped applied patches > > - added patch to amend dependencies (Mark) > > - amended the second patch accordingly (Mark) > > - elaborated purpose of the patch 6 in the commit message (Mark) > > > > [...] > > Applied to > > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next > > Thanks! > > [1/9] spi: pxa2xx: Narrow the Kconfig option visibility > commit: 3af201a405b3e5abee65102b062c309fff68cc0e > [2/9] spi: pxa2xx: Drop ACPI_PTR() and of_match_ptr() > commit: 9907c475dcab9b269422972577360122129ac84c > [3/9] spi: pxa2xx: Extract pxa2xx_spi_init_ssp() helper > commit: 7290f1e4075d28ab961df5a454503296fa289271 > [4/9] spi: pxa2xx: Skip SSP initialization if it's done elsewhere > commit: bb77c99ee6d3d704086acf141d3ec92601747809 Thank you! Do I need to do anything else to get the rest applied? -- With Best Regards, Andy Shevchenko
On Wed, Apr 03, 2024 at 02:07:29PM +0300, Andy Shevchenko wrote: > Do I need to do anything else to get the rest applied? All the concerns I have with swnodes just being a more complex and less maintainable way of doing things still stand, I'm not clear that this is making anything better.
On Wed, Apr 03, 2024 at 02:29:38PM +0100, Mark Brown wrote: > On Wed, Apr 03, 2024 at 02:07:29PM +0300, Andy Shevchenko wrote: > > > Do I need to do anything else to get the rest applied? > > All the concerns I have with swnodes just being a more complex and less > maintainable way of doing things still stand, I'm not clear that this is > making anything better. As I explained before it's not less maintainable than device tree sources. The only difference is that we don't have validation tool for in-kernel tables. And I don't see why we need that. The data describes the platforms and in the very same way may come to the driver from elsewhere. How would you validate that? It the same as we trust firmware (boot loader) or not. If we don't than how should we do at all? Can you point out what the exact aspect is most significant from C language perspective that we miss after conversion? Type checking? Something else? -- With Best Regards, Andy Shevchenko
On Wed, Apr 03, 2024 at 04:39:13PM +0300, Andy Shevchenko wrote: > On Wed, Apr 03, 2024 at 02:29:38PM +0100, Mark Brown wrote: > > On Wed, Apr 03, 2024 at 02:07:29PM +0300, Andy Shevchenko wrote: > > > > > Do I need to do anything else to get the rest applied? > > > > All the concerns I have with swnodes just being a more complex and less > > maintainable way of doing things still stand, I'm not clear that this is > > making anything better. > > As I explained before it's not less maintainable than device tree sources. > The only difference is that we don't have validation tool for in-kernel > tables. And I don't see why we need that. The data describes the platforms > and in the very same way may come to the driver from elsewhere. > How would you validate that? It the same as we trust firmware (boot loader) > or not. If we don't than how should we do at all? > > Can you point out what the exact aspect is most significant from C language > perspective that we miss after conversion? Type checking? Something else? Also note, after hiding the data structures from that file we open the door for the much bigger cleanup, and I have patches already precooked (need a bit of time to test, though). -- With Best Regards, Andy Shevchenko
On Wed, Apr 03, 2024 at 04:41:40PM +0300, Andy Shevchenko wrote: > On Wed, Apr 03, 2024 at 04:39:13PM +0300, Andy Shevchenko wrote: > > On Wed, Apr 03, 2024 at 02:29:38PM +0100, Mark Brown wrote: > > > All the concerns I have with swnodes just being a more complex and less > > > maintainable way of doing things still stand, I'm not clear that this is > > > making anything better. > > As I explained before it's not less maintainable than device tree sources. > > The only difference is that we don't have validation tool for in-kernel > > tables. And I don't see why we need that. The data describes the platforms > > and in the very same way may come to the driver from elsewhere. > > How would you validate that? It the same as we trust firmware (boot loader) > > or not. If we don't than how should we do at all? I don't think we should do this at all, all I see from these changes is effort in reviewing them - as far as I can tell they are a solution in search of a problem. DT has some support for validation of the formatting of the data supplied by the board and offers the potential for distributing the board description separately to the kernel. > > Can you point out what the exact aspect is most significant from C language > > perspective that we miss after conversion? Type checking? Something else? You're changing the code from supplying data from the board description via a simple C structure where the compiler offers at least some validation and where we can just supply directly usable values to an abstract data structure that's still encoded in C but has less validation and requires a bunch of code to parse at runtime. Platform data is unsurprisingly a good solution to the problem of supplying platform data. > Also note, after hiding the data structures from that file we open > the door for the much bigger cleanup, and I have patches already precooked > (need a bit of time to test, though). Perhaps those will motivate a change, as things stand I've not seen what you're proposing to do here.
On Wed, Apr 03, 2024 at 03:13:48PM +0100, Mark Brown wrote:
> On Wed, Apr 03, 2024 at 04:41:40PM +0300, Andy Shevchenko wrote:
> > On Wed, Apr 03, 2024 at 04:39:13PM +0300, Andy Shevchenko wrote:
> > > On Wed, Apr 03, 2024 at 02:29:38PM +0100, Mark Brown wrote:
>
> > > > All the concerns I have with swnodes just being a more complex and less
> > > > maintainable way of doing things still stand, I'm not clear that this is
> > > > making anything better.
>
> > > As I explained before it's not less maintainable than device tree sources.
> > > The only difference is that we don't have validation tool for in-kernel
> > > tables. And I don't see why we need that. The data describes the platforms
> > > and in the very same way may come to the driver from elsewhere.
> > > How would you validate that? It the same as we trust firmware (boot loader)
> > > or not. If we don't than how should we do at all?
>
> I don't think we should do this at all, all I see from these changes is
> effort in reviewing them - as far as I can tell they are a solution in
> search of a problem. DT has some support for validation of the
> formatting of the data supplied by the board and offers the potential
> for distributing the board description separately to the kernel.
>
> > > Can you point out what the exact aspect is most significant from C language
> > > perspective that we miss after conversion? Type checking? Something else?
>
> You're changing the code from supplying data from the board description
> via a simple C structure where the compiler offers at least some
> validation and where we can just supply directly usable values to an
> abstract data structure that's still encoded in C but has less
> validation and requires a bunch of code to parse at runtime. Platform
> data is unsurprisingly a good solution to the problem of supplying
> platform data.
Linus was long time ago against board files. Yet, we have a few old
(kinda supported) boards left in the tree. The conversion makes the
driver be prepared for the DT conversion when it happens. From maintenance
perspective my patch reduced the code under the maintenance, which reduces
time spent by both contributors and maintainers on this.
AFAIU all what you are moaning about is type checking. Okay, I got
it, but we have a lot of other places with similar approach done,
e.g. GPIO_LOOKUP*() tables that basically gives something unconnected to the
driver without any platform data being involved and you seems to be fine with
that:
$ git log --oneline --no-merges --grep 'Mark Brown' -- arch/ | grep 'GPIO desc'
I randomly took this 366f36e2a ("ASoC: wm1250-ev1: Convert to GPIO descriptors").
Can you tell how it is different to my proposal?
> > Also note, after hiding the data structures from that file we open
> > the door for the much bigger cleanup, and I have patches already precooked
> > (need a bit of time to test, though).
>
> Perhaps those will motivate a change, as things stand I've not seen what
> you're proposing to do here.
Okay, let me incorporate those into v3.
--
With Best Regards,
Andy Shevchenko
On Wed, Apr 03, 2024 at 05:41:30PM +0300, Andy Shevchenko wrote:
> Linus was long time ago against board files. Yet, we have a few old
> (kinda supported) boards left in the tree. The conversion makes the
> driver be prepared for the DT conversion when it happens. From maintenance
> perspective my patch reduced the code under the maintenance, which reduces
> time spent by both contributors and maintainers on this.
> AFAIU all what you are moaning about is type checking. Okay, I got
The type checking is part of it, but it's more a general taste thing
with using swnodes like this. You've not actually removed the board
file and it's hard to get enthusiastic about the change to the board
file that results, or to see this as a substantial step towards DT
conversion for the platform given the trivialness of the single
property here. As a general thing I don't want to encourage people to
start randomly converting things to swnode rather than to DT.
> it, but we have a lot of other places with similar approach done,
> e.g. GPIO_LOOKUP*() tables that basically gives something unconnected to the
> driver without any platform data being involved and you seems to be fine with
> that:
> $ git log --oneline --no-merges --grep 'Mark Brown' -- arch/ | grep 'GPIO desc'
> I randomly took this 366f36e2a ("ASoC: wm1250-ev1: Convert to GPIO descriptors").
> Can you tell how it is different to my proposal?
The main difference with the GPIO lookup tables is that they are
structured data specifically for GPIOs rather than the general purpose
free for all we have with swnode.
On Wed, Apr 03, 2024 at 04:50:00PM +0100, Mark Brown wrote:
> On Wed, Apr 03, 2024 at 05:41:30PM +0300, Andy Shevchenko wrote:
>
> > Linus was long time ago against board files. Yet, we have a few old
> > (kinda supported) boards left in the tree. The conversion makes the
> > driver be prepared for the DT conversion when it happens. From maintenance
> > perspective my patch reduced the code under the maintenance, which reduces
> > time spent by both contributors and maintainers on this.
>
> > AFAIU all what you are moaning about is type checking. Okay, I got
>
> The type checking is part of it, but it's more a general taste thing
> with using swnodes like this. You've not actually removed the board
> file and it's hard to get enthusiastic about the change to the board
> file that results, or to see this as a substantial step towards DT
> conversion for the platform given the trivialness of the single
> property here. As a general thing I don't want to encourage people to
> start randomly converting things to swnode rather than to DT.
Conversion to GPIO lookup tables is also the same in this sense.
But with it in place, the drivers aren't needed to be touched
when the real conversion happens. I agree, that _ideally_ we should
take that shot, but I am not an expert in DT and it will take a lot
for me to get to the shape, besides the fact of the ARM (platform)
specifics, which I'm far from. So, I prefer do step-by-step approach
if one developer can't fulfill the task. I.o.w. perfect is enemy of
good.
> > it, but we have a lot of other places with similar approach done,
> > e.g. GPIO_LOOKUP*() tables that basically gives something unconnected to the
> > driver without any platform data being involved and you seems to be fine with
> > that:
>
> > $ git log --oneline --no-merges --grep 'Mark Brown' -- arch/ | grep 'GPIO desc'
>
> > I randomly took this 366f36e2a ("ASoC: wm1250-ev1: Convert to GPIO descriptors").
>
> > Can you tell how it is different to my proposal?
>
> The main difference with the GPIO lookup tables is that they are
> structured data specifically for GPIOs rather than the general purpose
> free for all we have with swnode.
Semantically yes, technically they have all the same issues you pointed out
that swnode has.
Nevertheless, I'm about to send a part 2 of cleanup (I decided not mangle
this series, so it will be on top of this) for you to see how we can go
forward.
And thank you for this discussion.
--
With Best Regards,
Andy Shevchenko
© 2016 - 2025 Red Hat, Inc.