| Message ID | 20230818071405.28581-4-schmitzmic@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Q40 IDE fixes | expand |
Hi Michael, On Fri, Aug 18, 2023 at 9:14 AM Michael Schmitz <schmitzmic@gmail.com> wrote: > The pata_falcon data byte swapping patch relies on pdev->id > being 0 or 1. Q40 uses these IDs, but Atari used -1. Change > pdev->id to 0 for Atari Falcon IDE platform device so > selection of drive to byte-swap through pata_falcon.data_swab > can be used on Falcon as well. > Tested on ARAnyM so far. > > Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> Iff this patch is accepted, it must go in before [PATCH v2 2/3], else the latter would cause a regression. And backporting to stable must take that into account, too. Alternative, only consider pdev->id if it's positive? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, Am 19.08.2023 um 02:44 schrieb Geert Uytterhoeven: > Hi Michael, > > On Fri, Aug 18, 2023 at 9:14 AM Michael Schmitz <schmitzmic@gmail.com> wrote: >> The pata_falcon data byte swapping patch relies on pdev->id >> being 0 or 1. Q40 uses these IDs, but Atari used -1. Change >> pdev->id to 0 for Atari Falcon IDE platform device so >> selection of drive to byte-swap through pata_falcon.data_swab >> can be used on Falcon as well. >> Tested on ARAnyM so far. >> >> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> > > Iff this patch is accepted, it must go in before [PATCH v2 2/3], else > the latter would cause a regression. And backporting to stable must > take that into account, too. I don't see it as a regression - the driver still works OK, it's only the byte swap option that's broken on Atari, and that's newly introduced. But this patch changes the user space exposed platform device name, as you point out elsewhere. That's reason enough to drop it. > Alternative, only consider pdev->id if it's positive? Only shift by two bits if pdev->id is > 0? (some consider 0 positive...) I'll rewrite that bit and respin. Cheers, Michael > > Gr{oetje,eeting}s, > > Geert > > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds >
On Sat, 19 Aug 2023, Michael Schmitz wrote: > > > > Iff this patch is accepted, it must go in before [PATCH v2 2/3], else > > the latter would cause a regression. And backporting to stable must > > take that into account, too. > > I don't see it as a regression - the driver still works OK, it's only > the byte swap option that's broken on Atari, and that's newly > introduced. > > But this patch changes the user space exposed platform device name, as > you point out elsewhere. That's reason enough to drop it. > Such a script would be broken on q40 already. When a fix has a dependency on a separate patch you can backport both by specifying a cherry-picking sequence. We may need Geert to add those commit hashes though. This is discussed in Documentation/process/stable-kernel-rules.rst
Hi Finn, Am 19.08.2023 um 12:07 schrieb Finn Thain: > > On Sat, 19 Aug 2023, Michael Schmitz wrote: > >>> >>> Iff this patch is accepted, it must go in before [PATCH v2 2/3], else >>> the latter would cause a regression. And backporting to stable must >>> take that into account, too. >> >> I don't see it as a regression - the driver still works OK, it's only >> the byte swap option that's broken on Atari, and that's newly >> introduced. >> >> But this patch changes the user space exposed platform device name, as >> you point out elsewhere. That's reason enough to drop it. >> > > Such a script would be broken on q40 already. Not necessarily - what I thought of was something like the Debian installer. I don't know for a fact that it relies on the device naming as it is at present (or even that it supports Q40), but I'd rather not needlessly change something that might break user space code. > When a fix has a dependency on a separate patch you can backport both by > specifying a cherry-picking sequence. We may need Geert to add those > commit hashes though. This is discussed in > Documentation/process/stable-kernel-rules.rst If Geert prefers to change the Atari IDE platform device ID, he's always free to do that :-) The current patch will work either way. Cheers, Michael
diff --git a/arch/m68k/atari/config.c b/arch/m68k/atari/config.c index 38a7c0578105..e3e437cd0f84 100644 --- a/arch/m68k/atari/config.c +++ b/arch/m68k/atari/config.c @@ -925,7 +925,7 @@ int __init atari_platform_init(void) #endif if (ATARIHW_PRESENT(IDE)) { - pdev = platform_device_register_simple("atari-falcon-ide", -1, + pdev = platform_device_register_simple("atari-falcon-ide", 0, atari_falconide_rsrc, ARRAY_SIZE(atari_falconide_rsrc)); if (IS_ERR(pdev)) rv = PTR_ERR(pdev);
The pata_falcon data byte swapping patch relies on pdev->id being 0 or 1. Q40 uses these IDs, but Atari used -1. Change pdev->id to 0 for Atari Falcon IDE platform device so selection of drive to byte-swap through pata_falcon.data_swab can be used on Falcon as well. Tested on ARAnyM so far. Signed-off-by: Michael Schmitz <schmitzmic@gmail.com> --- arch/m68k/atari/config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)