Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
On Sun, 2018-12-30 at 18:50 +0100, LEROY Christophe wrote: > Arnd Bergmann a écrit : > > On Sat, Dec 29, 2018 at 3:51 AM Michael Schmitz > > wrote: [...] > > > (On second thought - I don't want to speculate whether there's > > > weird compiler options that could result in the > > > nvram_check_checksum and nvram_read_bytes symbols to still be > > > referenced in the final link, even though > > > IS_BUILTIN(CONFIG_NVRAM) always evaluates to false. Best > > > leave this as-is.) > > > > As far as I know, it's totally reliable with the supported > > compilers (gcc-4.6+). In the older compilers (e.g. 4.1), there was > > a corner case, where it could have failed to eliminate a function > > that was only referenced through a pointer from a discarded > > variable, but a plain IS_ENABLED() check like the one here > > was still ok, and lots of code relies on that. > > > > Other than that, I agree either way is totally fine here, so no > > objections to using the #ifdef. > > As far as I know, kernel codying style promotes the use of > IS_ENABLED() etc. instead of #ifdefs when possible. It's a preference, as with a lot of coding style stuff, which we leave up to the maintainer. That said, as has been pointed out, the current #ifdef has a failing corner case when both are modular (because the code should then be included). The runtime macro that correctly expresses this is IS_REACHABLE(CONFIG_NVRAM). James
Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
On Mon, 31 Dec 2018, Finn Thain wrote: > On Sun, 30 Dec 2018, James Bottomley wrote: > > > > > That said, as has been pointed out, the current #ifdef has a failing > > corner case when both are modular (because the code should then be > > included). The runtime macro that correctly expresses this is > > IS_REACHABLE(CONFIG_NVRAM). > > > > No, in the case of CONFIG_NVRAM=m, the conditional code is deliberately > excluded. This is discussed in the patch description. The convention on > PPC32 is that device drivers drop support for NVRAM in this situation. > > I've adopted the PPC32 convention here because M68K drivers and PPC32 > drivers have to co-exist (I'm thinking of valkyriefb, but there are other > examples). > I agree with your comment in principle, that these drivers should be using IS_REACHABLE(CONFIG_NVRAM), not defined(CONFIG_NVRAM). $ grep -lrw CONFIG_NVRAM drivers/ drivers/video/fbdev/matrox/matroxfb_base.c drivers/video/fbdev/platinumfb.c drivers/video/fbdev/controlfb.c drivers/video/fbdev/valkyriefb.c drivers/video/fbdev/imsttfb.c drivers/scsi/atari_scsi.c $ But I think this is not the fault of this patch; it's just a historical accident. Having said that, I will rework this series to convert all of the above drivers to IS_REACHABLE(CONFIG_NVRAM). I think it would be an improvement. Besides, it seems likely that some rework involving ppc_md will be needed too because as Arnd pointed out, it would be good to avoid two sets of nvram accessor methods. --
Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
On Sun, 30 Dec 2018, James Bottomley wrote: > > That said, as has been pointed out, the current #ifdef has a failing > corner case when both are modular (because the code should then be > included). The runtime macro that correctly expresses this is > IS_REACHABLE(CONFIG_NVRAM). > No, in the case of CONFIG_NVRAM=m, the conditional code is deliberately excluded. This is discussed in the patch description. The convention on PPC32 is that device drivers drop support for NVRAM in this situation. I've adopted the PPC32 convention here because M68K drivers and PPC32 drivers have to co-exist (I'm thinking of valkyriefb, but there are other examples). -- > James > >
Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
Arnd Bergmann a écrit : On Sat, Dec 29, 2018 at 3:51 AM Michael Schmitz wrote: Hi Finn, Am 29.12.2018 um 15:34 schrieb Finn Thain: > On Sat, 29 Dec 2018, Michael Schmitz wrote: > >> >> IS_BUILTIN(CONFIG_NVRAM) is probably what Christophe really meant to suggest. >> >> Or (really going out on a limb here): >> >> IS_BUILTIN(CONFIG_NVRAM) || >> ( IS_MODULE(CONFIG_ATARI_SCSI) && IS_ENABLED(CONFIG_NVRAM) ) >> >> Not that I'd advocate that, for this series. >> > > Well, you are a maintainer for atari_scsi.c. > > Are you saying that you want IS_BUILTIN(CONFIG_NVRAM) used here instead of > ifdef? No, just pointing out that there would be a way to avoid the ifdef without messing up driver behaviour. I'm fine with the ifdef - not least because it clearly eliminates code that would be unreachable. (On second thought - I don't want to speculate whether there's weird compiler options that could result in the nvram_check_checksum and nvram_read_bytes symbols to still be referenced in the final link, even though IS_BUILTIN(CONFIG_NVRAM) always evaluates to false. Best leave this as-is.) As far as I know, it's totally reliable with the supported compilers (gcc-4.6+). In the older compilers (e.g. 4.1), there was a corner case, where it could have failed to eliminate a function that was only referenced through a pointer from a discarded variable, but a plain IS_ENABLED() check like the one here was still ok, and lots of code relies on that. Other than that, I agree either way is totally fine here, so no objections to using the #ifdef. As far as I know, kernel codying style promotes the use of IS_ENABLED() etc. instead of #ifdefs when possible. Christophe Arnd
Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
On Sat, Dec 29, 2018 at 3:51 AM Michael Schmitz wrote: > > Hi Finn, > > Am 29.12.2018 um 15:34 schrieb Finn Thain: > > On Sat, 29 Dec 2018, Michael Schmitz wrote: > > > >> > >> IS_BUILTIN(CONFIG_NVRAM) is probably what Christophe really meant to > >> suggest. > >> > >> Or (really going out on a limb here): > >> > >> IS_BUILTIN(CONFIG_NVRAM) || > >> ( IS_MODULE(CONFIG_ATARI_SCSI) && IS_ENABLED(CONFIG_NVRAM) ) > >> > >> Not that I'd advocate that, for this series. > >> > > > > Well, you are a maintainer for atari_scsi.c. > > > > Are you saying that you want IS_BUILTIN(CONFIG_NVRAM) used here instead of > > ifdef? > > No, just pointing out that there would be a way to avoid the ifdef > without messing up driver behaviour. I'm fine with the ifdef - not least > because it clearly eliminates code that would be unreachable. > > (On second thought - I don't want to speculate whether there's weird > compiler options that could result in the nvram_check_checksum and > nvram_read_bytes symbols to still be referenced in the final link, even > though IS_BUILTIN(CONFIG_NVRAM) always evaluates to false. Best leave > this as-is.) As far as I know, it's totally reliable with the supported compilers (gcc-4.6+). In the older compilers (e.g. 4.1), there was a corner case, where it could have failed to eliminate a function that was only referenced through a pointer from a discarded variable, but a plain IS_ENABLED() check like the one here was still ok, and lots of code relies on that. Other than that, I agree either way is totally fine here, so no objections to using the #ifdef. Arnd
Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
Christophe,
Am 30.12.2018 um 05:55 schrieb LEROY Christophe:
Michael Schmitz a écrit :
Hi Finn,
Am 29.12.2018 um 14:06 schrieb Finn Thain:
On Fri, 28 Dec 2018, LEROY Christophe wrote:
diff --git a/drivers/scsi/atari_scsi.c b/drivers/scsi/atari_scsi.c
index 89f5154c40b6..99e5729d910d 100644
--- a/drivers/scsi/atari_scsi.c
+++ b/drivers/scsi/atari_scsi.c
@@ -755,9 +755,10 @@ static int __init atari_scsi_probe(struct
platform_device *pdev)
if (ATARIHW_PRESENT(TT_SCSI) && setup_sg_tablesize >= 0)
atari_scsi_template.sg_tablesize = setup_sg_tablesize;
-if (setup_hostid >= 0) {
+if (setup_hostid >= 0)
atari_scsi_template.this_id = setup_hostid & 7;
-} else {
+#ifdef CONFIG_NVRAM
+else
Such ifdefs should be avoided in C files.
It would be better to use
} else if (IS_ENABLED(CONFIG_NVRAM)) {
I don't like #ifdefs either. However, as the maintainer of this file,
I am
okay with this one.
The old #ifdef CONFIG_NVRAM conditional compilation convention that gets
used here and under drivers/video/fbdev could probably be improved upon
but I consider this to be out-of-scope for this series, which is
complicated enough.
And as explained in the commit log, CONFIG_NVRAM=y and CONFIG_NVRAM=m
are
treaded differently by drivers. Therefore, IS_ENABLED would be
incorrect.
IS_BUILTIN(CONFIG_NVRAM) is probably what Christophe really meant to
suggest.
Doesn't #ifdef means either y or m ? So the same as IS_ENABLED() ?
#ifdef CONFIG_NVRAM is used if you want to match CONFIG_NVRAM=y. For
CONFIG_NVRAM=m, you'd use #ifdef CONFIG_NVRAM_MODULE.
Cheers,
Michael
Christophe
Or (really going out on a limb here):
IS_BUILTIN(CONFIG_NVRAM) ||
( IS_MODULE(CONFIG_ATARI_SCSI) && IS_ENABLED(CONFIG_NVRAM) )
Not that I'd advocate that, for this series.
Cheers,
Michael
Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
Michael Schmitz a écrit :
Hi Finn,
Am 29.12.2018 um 14:06 schrieb Finn Thain:
On Fri, 28 Dec 2018, LEROY Christophe wrote:
diff --git a/drivers/scsi/atari_scsi.c b/drivers/scsi/atari_scsi.c
index 89f5154c40b6..99e5729d910d 100644
--- a/drivers/scsi/atari_scsi.c
+++ b/drivers/scsi/atari_scsi.c
@@ -755,9 +755,10 @@ static int __init atari_scsi_probe(struct
platform_device *pdev)
if (ATARIHW_PRESENT(TT_SCSI) && setup_sg_tablesize >= 0)
atari_scsi_template.sg_tablesize = setup_sg_tablesize;
- if (setup_hostid >= 0) {
+ if (setup_hostid >= 0)
atari_scsi_template.this_id = setup_hostid & 7;
- } else {
+#ifdef CONFIG_NVRAM
+ else
Such ifdefs should be avoided in C files.
It would be better to use
} else if (IS_ENABLED(CONFIG_NVRAM)) {
I don't like #ifdefs either. However, as the maintainer of this file, I am
okay with this one.
The old #ifdef CONFIG_NVRAM conditional compilation convention that gets
used here and under drivers/video/fbdev could probably be improved upon
but I consider this to be out-of-scope for this series, which is
complicated enough.
And as explained in the commit log, CONFIG_NVRAM=y and CONFIG_NVRAM=m are
treaded differently by drivers. Therefore, IS_ENABLED would be incorrect.
IS_BUILTIN(CONFIG_NVRAM) is probably what Christophe really meant to suggest.
Doesn't #ifdef means either y or m ? So the same as IS_ENABLED() ?
Christophe
Or (really going out on a limb here):
IS_BUILTIN(CONFIG_NVRAM) ||
( IS_MODULE(CONFIG_ATARI_SCSI) && IS_ENABLED(CONFIG_NVRAM) )
Not that I'd advocate that, for this series.
Cheers,
Michael
Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
Hi Finn,
Am 26.12.2018 um 13:37 schrieb Finn Thain:
On powerpc, setting CONFIG_NVRAM=n builds a kernel with no NVRAM support.
Setting CONFIG_NVRAM=m enables the /dev/nvram misc device module without
enabling NVRAM support in drivers. Setting CONFIG_NVRAM=y enables the
misc device (built-in) and also enables NVRAM support in drivers.
m68k shares the valkyriefb driver with powerpc, and since that driver uses
NVRAM, it is affected by CONFIG_ATARI_SCSI, because of the use of
"select NVRAM".
Adopt the powerpc convention on m68k to avoid surprises.
Signed-off-by: Finn Thain
Tested-by: Christian T. Steigies
Acked-by: Michael Schmitz
---
This patch temporarily disables CONFIG_NVRAM on Atari, to prevent build
failures when bisecting the rest of this patch series. It gets enabled
again with the introduction of CONFIG_HAVE_ARCH_NVRAM_OPS, once the
nvram_* global functions have been moved to an ops struct.
---
drivers/char/Kconfig | 5 +
drivers/scsi/Kconfig | 6 +++---
drivers/scsi/atari_scsi.c | 7 ---
3 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 9d03b2ff5df6..5b54595dfe30 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -236,7 +236,7 @@ source "drivers/char/hw_random/Kconfig"
config NVRAM
tristate "/dev/nvram support"
- depends on ATARI || X86 || GENERIC_NVRAM
+ depends on X86 || GENERIC_NVRAM
---help---
If you say Y here and create a character special file /dev/nvram
with major number 10 and minor number 144 using mknod ("man mknod"),
@@ -254,9 +254,6 @@ config NVRAM
should NEVER idly tamper with it. See Ralf Brown's interrupt list
for a guide to the use of CMOS bytes by your BIOS.
- On Atari machines, /dev/nvram is always configured and does not need
- to be selected.
-
To compile this driver as a module, choose M here: the
module will be called nvram.
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 640cd1b31a18..924eb69e7fc4 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -1381,14 +1381,14 @@ config ATARI_SCSI
tristate "Atari native SCSI support"
depends on ATARI && SCSI
select SCSI_SPI_ATTRS
- select NVRAM
---help---
If you have an Atari with built-in NCR5380 SCSI controller (TT,
Falcon, ...) say Y to get it supported. Of course also, if you have
a compatible SCSI controller (e.g. for Medusa).
- To compile this driver as a module, choose M here: the
- module will be called atari_scsi.
+ To compile this driver as a module, choose M here: the module will
+ be called atari_scsi. If you also enable NVRAM support, the SCSI
+ host's ID is taken from the setting in TT RTC NVRAM.
This driver supports both styles of NCR integration into the
system: the TT style (separate DMA), and the Falcon style (via
diff --git a/drivers/scsi/atari_scsi.c b/drivers/scsi/atari_scsi.c
index 89f5154c40b6..99e5729d910d 100644
--- a/drivers/scsi/atari_scsi.c
+++ b/drivers/scsi/atari_scsi.c
@@ -755,9 +755,10 @@ static int __init atari_scsi_probe(struct platform_device
*pdev)
if (ATARIHW_PRESENT(TT_SCSI) && setup_sg_tablesize >= 0)
atari_scsi_template.sg_tablesize = setup_sg_tablesize;
- if (setup_hostid >= 0) {
+ if (setup_hostid >= 0)
atari_scsi_template.this_id = setup_hostid & 7;
- } else {
+#ifdef CONFIG_NVRAM
+ else
/* Test if a host id is set in the NVRam */
if (ATARIHW_PRESENT(TT_CLK) && nvram_check_checksum()) {
unsigned char b = nvram_read_byte(16);
@@ -768,7 +769,7 @@ static int __init atari_scsi_probe(struct platform_device
*pdev)
if (b & 0x80)
atari_scsi_template.this_id = b & 7;
}
- }
+#endif
/* If running on a Falcon and if there's TT-Ram (i.e., more than one
* memory block, since there's always ST-Ram in a Falcon), then
Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
Hi Finn, Am 29.12.2018 um 15:34 schrieb Finn Thain: On Sat, 29 Dec 2018, Michael Schmitz wrote: IS_BUILTIN(CONFIG_NVRAM) is probably what Christophe really meant to suggest. Or (really going out on a limb here): IS_BUILTIN(CONFIG_NVRAM) || ( IS_MODULE(CONFIG_ATARI_SCSI) && IS_ENABLED(CONFIG_NVRAM) ) Not that I'd advocate that, for this series. Well, you are a maintainer for atari_scsi.c. Are you saying that you want IS_BUILTIN(CONFIG_NVRAM) used here instead of ifdef? No, just pointing out that there would be a way to avoid the ifdef without messing up driver behaviour. I'm fine with the ifdef - not least because it clearly eliminates code that would be unreachable. (On second thought - I don't want to speculate whether there's weird compiler options that could result in the nvram_check_checksum and nvram_read_bytes symbols to still be referenced in the final link, even though IS_BUILTIN(CONFIG_NVRAM) always evaluates to false. Best leave this as-is.) OTOH, if you approve of the existing patch, please send your acked-by. Of course - I'd seen Geert's acked-by on some of the patches and forgot to check which still required acks. Cheers, Michael
Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
On Sat, 29 Dec 2018, Michael Schmitz wrote: > > IS_BUILTIN(CONFIG_NVRAM) is probably what Christophe really meant to suggest. > > Or (really going out on a limb here): > > IS_BUILTIN(CONFIG_NVRAM) || > ( IS_MODULE(CONFIG_ATARI_SCSI) && IS_ENABLED(CONFIG_NVRAM) ) > > Not that I'd advocate that, for this series. > Well, you are a maintainer for atari_scsi.c. Are you saying that you want IS_BUILTIN(CONFIG_NVRAM) used here instead of ifdef? OTOH, if you approve of the existing patch, please send your acked-by. -- > Cheers, > > Michael > > > >
Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
Hi Finn,
Am 29.12.2018 um 14:06 schrieb Finn Thain:
On Fri, 28 Dec 2018, LEROY Christophe wrote:
diff --git a/drivers/scsi/atari_scsi.c b/drivers/scsi/atari_scsi.c
index 89f5154c40b6..99e5729d910d 100644
--- a/drivers/scsi/atari_scsi.c
+++ b/drivers/scsi/atari_scsi.c
@@ -755,9 +755,10 @@ static int __init atari_scsi_probe(struct
platform_device *pdev)
if (ATARIHW_PRESENT(TT_SCSI) && setup_sg_tablesize >= 0)
atari_scsi_template.sg_tablesize = setup_sg_tablesize;
- if (setup_hostid >= 0) {
+ if (setup_hostid >= 0)
atari_scsi_template.this_id = setup_hostid & 7;
- } else {
+#ifdef CONFIG_NVRAM
+ else
Such ifdefs should be avoided in C files.
It would be better to use
} else if (IS_ENABLED(CONFIG_NVRAM)) {
I don't like #ifdefs either. However, as the maintainer of this file, I am
okay with this one.
The old #ifdef CONFIG_NVRAM conditional compilation convention that gets
used here and under drivers/video/fbdev could probably be improved upon
but I consider this to be out-of-scope for this series, which is
complicated enough.
And as explained in the commit log, CONFIG_NVRAM=y and CONFIG_NVRAM=m are
treaded differently by drivers. Therefore, IS_ENABLED would be incorrect.
IS_BUILTIN(CONFIG_NVRAM) is probably what Christophe really meant to
suggest.
Or (really going out on a limb here):
IS_BUILTIN(CONFIG_NVRAM) ||
( IS_MODULE(CONFIG_ATARI_SCSI) && IS_ENABLED(CONFIG_NVRAM) )
Not that I'd advocate that, for this series.
Cheers,
Michael
Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
On Fri, 28 Dec 2018, LEROY Christophe wrote:
> Finn Thain a ?crit?:
>
> > On powerpc, setting CONFIG_NVRAM=n builds a kernel with no NVRAM support.
> > Setting CONFIG_NVRAM=m enables the /dev/nvram misc device module without
> > enabling NVRAM support in drivers. Setting CONFIG_NVRAM=y enables the
> > misc device (built-in) and also enables NVRAM support in drivers.
> >
> > m68k shares the valkyriefb driver with powerpc, and since that driver uses
> > NVRAM, it is affected by CONFIG_ATARI_SCSI, because of the use of
> > "select NVRAM".
> >
> > Adopt the powerpc convention on m68k to avoid surprises.
> >
> > Signed-off-by: Finn Thain
> > Tested-by: Christian T. Steigies
> > ---
> > This patch temporarily disables CONFIG_NVRAM on Atari, to prevent build
> > failures when bisecting the rest of this patch series. It gets enabled
> > again with the introduction of CONFIG_HAVE_ARCH_NVRAM_OPS, once the
> > nvram_* global functions have been moved to an ops struct.
> > ---
> > drivers/char/Kconfig | 5 +
> > drivers/scsi/Kconfig | 6 +++---
> > drivers/scsi/atari_scsi.c | 7 ---
> > 3 files changed, 8 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
> > index 9d03b2ff5df6..5b54595dfe30 100644
> > --- a/drivers/char/Kconfig
> > +++ b/drivers/char/Kconfig
> > @@ -236,7 +236,7 @@ source "drivers/char/hw_random/Kconfig"
> >
> > config NVRAM
> > tristate "/dev/nvram support"
> > - depends on ATARI || X86 || GENERIC_NVRAM
> > + depends on X86 || GENERIC_NVRAM
> > ---help---
> > If you say Y here and create a character special file /dev/nvram
> > with major number 10 and minor number 144 using mknod ("man mknod"),
> > @@ -254,9 +254,6 @@ config NVRAM
> > should NEVER idly tamper with it. See Ralf Brown's interrupt list
> > for a guide to the use of CMOS bytes by your BIOS.
> >
> > - On Atari machines, /dev/nvram is always configured and does not need
> > - to be selected.
> > -
> > To compile this driver as a module, choose M here: the
> > module will be called nvram.
> >
> > diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> > index 640cd1b31a18..924eb69e7fc4 100644
> > --- a/drivers/scsi/Kconfig
> > +++ b/drivers/scsi/Kconfig
> > @@ -1381,14 +1381,14 @@ config ATARI_SCSI
> > tristate "Atari native SCSI support"
> > depends on ATARI && SCSI
> > select SCSI_SPI_ATTRS
> > - select NVRAM
> > ---help---
> > If you have an Atari with built-in NCR5380 SCSI controller (TT,
> > Falcon, ...) say Y to get it supported. Of course also, if you have
> > a compatible SCSI controller (e.g. for Medusa).
> >
> > - To compile this driver as a module, choose M here: the
> > - module will be called atari_scsi.
> > + To compile this driver as a module, choose M here: the module will
> > + be called atari_scsi. If you also enable NVRAM support, the SCSI
> > + host's ID is taken from the setting in TT RTC NVRAM.
> >
> > This driver supports both styles of NCR integration into the
> > system: the TT style (separate DMA), and the Falcon style (via
> > diff --git a/drivers/scsi/atari_scsi.c b/drivers/scsi/atari_scsi.c
> > index 89f5154c40b6..99e5729d910d 100644
> > --- a/drivers/scsi/atari_scsi.c
> > +++ b/drivers/scsi/atari_scsi.c
> > @@ -755,9 +755,10 @@ static int __init atari_scsi_probe(struct
> > platform_device *pdev)
> > if (ATARIHW_PRESENT(TT_SCSI) && setup_sg_tablesize >= 0)
> > atari_scsi_template.sg_tablesize = setup_sg_tablesize;
> >
> > - if (setup_hostid >= 0) {
> > + if (setup_hostid >= 0)
> > atari_scsi_template.this_id = setup_hostid & 7;
> > - } else {
> > +#ifdef CONFIG_NVRAM
> > + else
>
> Such ifdefs should be avoided in C files.
> It would be better to use
>
> } else if (IS_ENABLED(CONFIG_NVRAM)) {
>
I don't like #ifdefs either. However, as the maintainer of this file, I am
okay with this one.
The old #ifdef CONFIG_NVRAM conditional compilation convention that gets
used here and under drivers/video/fbdev could probably be improved upon
but I consider this to be out-of-scope for this series, which is
complicated enough.
And as explained in the commit log, CONFIG_NVRAM=y and CONFIG_NVRAM=m are
treaded differently by drivers. Therefore, IS_ENABLED would be incorrect.
--
> > /* Test if a host id is set in the NVRam */
> > if (ATARIHW_PRESENT(TT_CLK) && nvram_check_checksum()) {
> > unsigned char b = nvram_read_byte(16);
> > @@ -768,7 +769,7 @@ static int __init atari_scsi_probe(struct
> > platform_device *pdev)
> > if (b & 0x80)
> > atari_scsi_template.this_id = b & 7;
> > }
> > - }
> > +#endif
> >
> > /* If running on a Falcon and if there's TT-Ram (i.e., more than one
> > * memory block, since there's always ST-Ram in a Falcon), then
> > --
> > 2.19.2
>
>
>
Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
Finn Thain a écrit :
On powerpc, setting CONFIG_NVRAM=n builds a kernel with no NVRAM support.
Setting CONFIG_NVRAM=m enables the /dev/nvram misc device module without
enabling NVRAM support in drivers. Setting CONFIG_NVRAM=y enables the
misc device (built-in) and also enables NVRAM support in drivers.
m68k shares the valkyriefb driver with powerpc, and since that driver uses
NVRAM, it is affected by CONFIG_ATARI_SCSI, because of the use of
"select NVRAM".
Adopt the powerpc convention on m68k to avoid surprises.
Signed-off-by: Finn Thain
Tested-by: Christian T. Steigies
---
This patch temporarily disables CONFIG_NVRAM on Atari, to prevent build
failures when bisecting the rest of this patch series. It gets enabled
again with the introduction of CONFIG_HAVE_ARCH_NVRAM_OPS, once the
nvram_* global functions have been moved to an ops struct.
---
drivers/char/Kconfig | 5 +
drivers/scsi/Kconfig | 6 +++---
drivers/scsi/atari_scsi.c | 7 ---
3 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 9d03b2ff5df6..5b54595dfe30 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -236,7 +236,7 @@ source "drivers/char/hw_random/Kconfig"
config NVRAM
tristate "/dev/nvram support"
- depends on ATARI || X86 || GENERIC_NVRAM
+ depends on X86 || GENERIC_NVRAM
---help---
If you say Y here and create a character special file /dev/nvram
with major number 10 and minor number 144 using mknod ("man mknod"),
@@ -254,9 +254,6 @@ config NVRAM
should NEVER idly tamper with it. See Ralf Brown's interrupt list
for a guide to the use of CMOS bytes by your BIOS.
- On Atari machines, /dev/nvram is always configured and does not need
- to be selected.
-
To compile this driver as a module, choose M here: the
module will be called nvram.
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 640cd1b31a18..924eb69e7fc4 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -1381,14 +1381,14 @@ config ATARI_SCSI
tristate "Atari native SCSI support"
depends on ATARI && SCSI
select SCSI_SPI_ATTRS
- select NVRAM
---help---
If you have an Atari with built-in NCR5380 SCSI controller (TT,
Falcon, ...) say Y to get it supported. Of course also, if you have
a compatible SCSI controller (e.g. for Medusa).
- To compile this driver as a module, choose M here: the
- module will be called atari_scsi.
+ To compile this driver as a module, choose M here: the module will
+ be called atari_scsi. If you also enable NVRAM support, the SCSI
+ host's ID is taken from the setting in TT RTC NVRAM.
This driver supports both styles of NCR integration into the
system: the TT style (separate DMA), and the Falcon style (via
diff --git a/drivers/scsi/atari_scsi.c b/drivers/scsi/atari_scsi.c
index 89f5154c40b6..99e5729d910d 100644
--- a/drivers/scsi/atari_scsi.c
+++ b/drivers/scsi/atari_scsi.c
@@ -755,9 +755,10 @@ static int __init atari_scsi_probe(struct
platform_device *pdev)
if (ATARIHW_PRESENT(TT_SCSI) && setup_sg_tablesize >= 0)
atari_scsi_template.sg_tablesize = setup_sg_tablesize;
- if (setup_hostid >= 0) {
+ if (setup_hostid >= 0)
atari_scsi_template.this_id = setup_hostid & 7;
- } else {
+#ifdef CONFIG_NVRAM
+ else
Such ifdefs should be avoided in C files.
It would be better to use
} else if (IS_ENABLED(CONFIG_NVRAM)) {
/* Test if a host id is set in the NVRam */
if (ATARIHW_PRESENT(TT_CLK) && nvram_check_checksum()) {
unsigned char b = nvram_read_byte(16);
@@ -768,7 +769,7 @@ static int __init atari_scsi_probe(struct
platform_device *pdev)
if (b & 0x80)
atari_scsi_template.this_id = b & 7;
}
- }
+#endif
/* If running on a Falcon and if there's TT-Ram (i.e., more than one
* memory block, since there's always ST-Ram in a Falcon), then
--
2.19.2
[PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
On powerpc, setting CONFIG_NVRAM=n builds a kernel with no NVRAM support.
Setting CONFIG_NVRAM=m enables the /dev/nvram misc device module without
enabling NVRAM support in drivers. Setting CONFIG_NVRAM=y enables the
misc device (built-in) and also enables NVRAM support in drivers.
m68k shares the valkyriefb driver with powerpc, and since that driver uses
NVRAM, it is affected by CONFIG_ATARI_SCSI, because of the use of
"select NVRAM".
Adopt the powerpc convention on m68k to avoid surprises.
Signed-off-by: Finn Thain
Tested-by: Christian T. Steigies
---
This patch temporarily disables CONFIG_NVRAM on Atari, to prevent build
failures when bisecting the rest of this patch series. It gets enabled
again with the introduction of CONFIG_HAVE_ARCH_NVRAM_OPS, once the
nvram_* global functions have been moved to an ops struct.
---
drivers/char/Kconfig | 5 +
drivers/scsi/Kconfig | 6 +++---
drivers/scsi/atari_scsi.c | 7 ---
3 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 9d03b2ff5df6..5b54595dfe30 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -236,7 +236,7 @@ source "drivers/char/hw_random/Kconfig"
config NVRAM
tristate "/dev/nvram support"
- depends on ATARI || X86 || GENERIC_NVRAM
+ depends on X86 || GENERIC_NVRAM
---help---
If you say Y here and create a character special file /dev/nvram
with major number 10 and minor number 144 using mknod ("man mknod"),
@@ -254,9 +254,6 @@ config NVRAM
should NEVER idly tamper with it. See Ralf Brown's interrupt list
for a guide to the use of CMOS bytes by your BIOS.
- On Atari machines, /dev/nvram is always configured and does not need
- to be selected.
-
To compile this driver as a module, choose M here: the
module will be called nvram.
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 640cd1b31a18..924eb69e7fc4 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -1381,14 +1381,14 @@ config ATARI_SCSI
tristate "Atari native SCSI support"
depends on ATARI && SCSI
select SCSI_SPI_ATTRS
- select NVRAM
---help---
If you have an Atari with built-in NCR5380 SCSI controller (TT,
Falcon, ...) say Y to get it supported. Of course also, if you have
a compatible SCSI controller (e.g. for Medusa).
- To compile this driver as a module, choose M here: the
- module will be called atari_scsi.
+ To compile this driver as a module, choose M here: the module will
+ be called atari_scsi. If you also enable NVRAM support, the SCSI
+ host's ID is taken from the setting in TT RTC NVRAM.
This driver supports both styles of NCR integration into the
system: the TT style (separate DMA), and the Falcon style (via
diff --git a/drivers/scsi/atari_scsi.c b/drivers/scsi/atari_scsi.c
index 89f5154c40b6..99e5729d910d 100644
--- a/drivers/scsi/atari_scsi.c
+++ b/drivers/scsi/atari_scsi.c
@@ -755,9 +755,10 @@ static int __init atari_scsi_probe(struct platform_device
*pdev)
if (ATARIHW_PRESENT(TT_SCSI) && setup_sg_tablesize >= 0)
atari_scsi_template.sg_tablesize = setup_sg_tablesize;
- if (setup_hostid >= 0) {
+ if (setup_hostid >= 0)
atari_scsi_template.this_id = setup_hostid & 7;
- } else {
+#ifdef CONFIG_NVRAM
+ else
/* Test if a host id is set in the NVRam */
if (ATARIHW_PRESENT(TT_CLK) && nvram_check_checksum()) {
unsigned char b = nvram_read_byte(16);
@@ -768,7 +769,7 @@ static int __init atari_scsi_probe(struct platform_device
*pdev)
if (b & 0x80)
atari_scsi_template.this_id = b & 7;
}
- }
+#endif
/* If running on a Falcon and if there's TT-Ram (i.e., more than one
* memory block, since there's always ST-Ram in a Falcon), then
--
2.19.2
