[PATCH 2/2] ide-generic: added messages in case of PCI IDE devices

[PATCH 2/2] ide-generic: added messages in case of PCI IDE devices

[Luiz Carlos Ramos]

Two messages are now displayed (at "warn" level) to make sure the user
is aware that PCI IDE devices were detected at common addresses (0x1f0
and 0x170) and that they will not be managed by ide-generic, but rather
delegated to its specific driver.

Signed-off-by: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
---
 drivers/ide/ide-generic.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/ide/ide-generic.c b/drivers/ide/ide-generic.c
index ee11edcdba3170c077381d603918498d79ffa3bb..2204819ef73612deed4ebfe58c69cdadb40fa53b 100644
--- a/drivers/ide/ide-generic.c
+++ b/drivers/ide/ide-generic.c
@@ -125,9 +125,19 @@ static int __init ide_generic_init(void)
 
 		if (primary == 0)
 			probe_mask |= 0x1;
+		else {
+			printk(KERN_WARN DRV_NAME ": PCI IDE device detected at 0x%4.4x."
+			     " ide-generic will not grab this device, delegating it to"
+			     " the driver of its specific chipset\n", 0x1f0);
+		}
 
 		if (secondary == 0)
 			probe_mask |= 0x2;
+		else {
+			printk(KERN_WARN DRV_NAME ": PCI IDE device detected at 0x%4.4x."
+			     " ide-generic will not grab this device, delegating it to"
+			     " the driver of its specific chipset\n", 0x170);
+		}
 	} else
 		printk(KERN_INFO DRV_NAME ": enforcing probing of I/O ports "
 			"upon user request\n");
-- 
2.8.2

Re: [PATCH 2/2] ide-generic: added messages in case of PCI IDE devices

[Sergei Shtylyov]

Hello.

On 02/21/2017 05:22 PM, Luiz Carlos Ramos wrote:

> Two messages are now displayed (at "warn" level) to make sure the user
> is aware that PCI IDE devices were detected at common addresses (0x1f0
> and 0x170) and that they will not be managed by ide-generic, but rather
> delegated to its specific driver.
>
> Signed-off-by: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
> ---
>  drivers/ide/ide-generic.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/ide/ide-generic.c b/drivers/ide/ide-generic.c
> index ee11edcdba3170c077381d603918498d79ffa3bb..2204819ef73612deed4ebfe58c69cdadb40fa53b 100644
> --- a/drivers/ide/ide-generic.c
> +++ b/drivers/ide/ide-generic.c
> @@ -125,9 +125,19 @@ static int __init ide_generic_init(void)
>
>  		if (primary == 0)
>  			probe_mask |= 0x1;
> +		else {

    CodingStyle: need {} in both *if* branches if at least one branch has them.

> +			printk(KERN_WARN DRV_NAME ": PCI IDE device detected at 0x%4.4x."
> +			     " ide-generic will not grab this device, delegating it to"
> +			     " the driver of its specific chipset\n", 0x1f0);

    The messages shouldn't be split -- checkpatch.pl knows about that.

> +		}
>
>  		if (secondary == 0)
>  			probe_mask |= 0x2;
> +		else {
> +			printk(KERN_WARN DRV_NAME ": PCI IDE device detected at 0x%4.4x."
> +			     " ide-generic will not grab this device, delegating it to"
> +			     " the driver of its specific chipset\n", 0x170);
> +		}

    Same here.

[..]

MBR, Sergei

Re: [PATCH 2/2] ide-generic: added messages in case of PCI IDE devices

[Sergei Shtylyov]

On 02/21/2017 05:22 PM, Luiz Carlos Ramos wrote:

> Two messages are now displayed (at "warn" level) to make sure the user
> is aware that PCI IDE devices were detected at common addresses (0x1f0
> and 0x170) and that they will not be managed by ide-generic, but rather
> delegated to its specific driver.
>
> Signed-off-by: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
> ---
>  drivers/ide/ide-generic.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/ide/ide-generic.c b/drivers/ide/ide-generic.c
> index ee11edcdba3170c077381d603918498d79ffa3bb..2204819ef73612deed4ebfe58c69cdadb40fa53b 100644
> --- a/drivers/ide/ide-generic.c
> +++ b/drivers/ide/ide-generic.c
> @@ -125,9 +125,19 @@ static int __init ide_generic_init(void)
>
>  		if (primary == 0)
>  			probe_mask |= 0x1;
> +		else {
> +			printk(KERN_WARN DRV_NAME ": PCI IDE device detected at 0x%4.4x."

    Also, please use pr_warn().

> +			     " ide-generic will not grab this device, delegating it to"
> +			     " the driver of its specific chipset\n", 0x1f0);
> +		}
>
>  		if (secondary == 0)
>  			probe_mask |= 0x2;
> +		else {
> +			printk(KERN_WARN DRV_NAME ": PCI IDE device detected at 0x%4.4x."

    Likewise.

> +			     " ide-generic will not grab this device, delegating it to"
> +			     " the driver of its specific chipset\n", 0x170);
> +		}
>  	} else
>  		printk(KERN_INFO DRV_NAME ": enforcing probing of I/O ports "
>  			"upon user request\n");

MBR, Sergei

Re: [PATCH 2/2] ide-generic: added messages in case of PCI IDE devices

[Luiz Carlos Ramos]

Hello, Sergei!

I have a question or two about checkpatch.pl results below.

On Tue, Feb 21, 2017 at 06:52:13PM +0300, Sergei Shtylyov wrote:
> On 02/21/2017 05:22 PM, Luiz Carlos Ramos wrote:
> 
> >Two messages are now displayed (at "warn" level) to make sure the user
> >is aware that PCI IDE devices were detected at common addresses (0x1f0
> >and 0x170) and that they will not be managed by ide-generic, but rather
> >delegated to its specific driver.
> >
> >Signed-off-by: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
> >---
> > drivers/ide/ide-generic.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> >diff --git a/drivers/ide/ide-generic.c b/drivers/ide/ide-generic.c
> >index ee11edcdba3170c077381d603918498d79ffa3bb..2204819ef73612deed4ebfe58c69cdadb40fa53b 100644
> >--- a/drivers/ide/ide-generic.c
> >+++ b/drivers/ide/ide-generic.c
> >@@ -125,9 +125,19 @@ static int __init ide_generic_init(void)
> >
> > 		if (primary == 0)
> > 			probe_mask |= 0x1;
> >+		else {
> >+			printk(KERN_WARN DRV_NAME ": PCI IDE device detected at 0x%4.4x."
> 
>    Also, please use pr_warn().
> 

This code is quite old, and it is in deep maintenance phase.

Every other message in ide-generic.c is implemented as printk(...),
but to be fair, there are not so many of them.

Given that fact, would it be better to change these two ones to be
inserted to pr_warn() as suggested (and then having a mix of printk()
and pr_warn() in the code), or to "allow" two more printk() in the code
and having checkpatck.pl complaining?

I really don't know what is better or worse. Both can be implemented,
and there are a mix of advantages and disadvantages in each side.

I'd like to hear from you and colleagues about how to proceed.

> >+			     " ide-generic will not grab this device, delegating it to"
> >+			     " the driver of its specific chipset\n", 0x1f0);
> >+		}
> >
> > 		if (secondary == 0)
> > 			probe_mask |= 0x2;
> >+		else {
> >+			printk(KERN_WARN DRV_NAME ": PCI IDE device detected at 0x%4.4x."
> 
>    Likewise.
> 
> >+			     " ide-generic will not grab this device, delegating it to"
> >+			     " the driver of its specific chipset\n", 0x170);
> >+		}
> > 	} else
> > 		printk(KERN_INFO DRV_NAME ": enforcing probing of I/O ports "
> > 			"upon user request\n");
> 
> MBR, Sergei
> 

Best regards,

Luiz Carlos Ramos

Re: [PATCH 2/2] ide-generic: added messages in case of PCI IDE devices

[Bartlomiej Zolnierkiewicz]

On Tuesday, February 21, 2017 09:38:31 PM Luiz Carlos Ramos wrote:
> Hello, Sergei!
> 
> I have a question or two about checkpatch.pl results below.
> 
> On Tue, Feb 21, 2017 at 06:52:13PM +0300, Sergei Shtylyov wrote:
> > On 02/21/2017 05:22 PM, Luiz Carlos Ramos wrote:
> > 
> > >Two messages are now displayed (at "warn" level) to make sure the user
> > >is aware that PCI IDE devices were detected at common addresses (0x1f0
> > >and 0x170) and that they will not be managed by ide-generic, but rather
> > >delegated to its specific driver.
> > >
> > >Signed-off-by: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
> > >---
> > > drivers/ide/ide-generic.c | 10 ++++++++++
> > > 1 file changed, 10 insertions(+)
> > >
> > >diff --git a/drivers/ide/ide-generic.c b/drivers/ide/ide-generic.c
> > >index ee11edcdba3170c077381d603918498d79ffa3bb..2204819ef73612deed4ebfe58c69cdadb40fa53b 100644
> > >--- a/drivers/ide/ide-generic.c
> > >+++ b/drivers/ide/ide-generic.c
> > >@@ -125,9 +125,19 @@ static int __init ide_generic_init(void)
> > >
> > > 		if (primary == 0)
> > > 			probe_mask |= 0x1;
> > >+		else {
> > >+			printk(KERN_WARN DRV_NAME ": PCI IDE device detected at 0x%4.4x."
> > 
> >    Also, please use pr_warn().
> > 
> 
> This code is quite old, and it is in deep maintenance phase.
> 
> Every other message in ide-generic.c is implemented as printk(...),
> but to be fair, there are not so many of them.
> 
> Given that fact, would it be better to change these two ones to be
> inserted to pr_warn() as suggested (and then having a mix of printk()
> and pr_warn() in the code), or to "allow" two more printk() in the code
> and having checkpatck.pl complaining?
> 
> I really don't know what is better or worse. Both can be implemented,
> and there are a mix of advantages and disadvantages in each side.
> 
> I'd like to hear from you and colleagues about how to proceed.

Well, I vote for allowing two more printk() and leaving
the old code as it is.

> > >+			     " ide-generic will not grab this device, delegating it to"
> > >+			     " the driver of its specific chipset\n", 0x1f0);
> > >+		}
> > >
> > > 		if (secondary == 0)
> > > 			probe_mask |= 0x2;
> > >+		else {
> > >+			printk(KERN_WARN DRV_NAME ": PCI IDE device detected at 0x%4.4x."
> > 
> >    Likewise.
> > 
> > >+			     " ide-generic will not grab this device, delegating it to"
> > >+			     " the driver of its specific chipset\n", 0x170);
> > >+		}
> > > 	} else
> > > 		printk(KERN_INFO DRV_NAME ": enforcing probing of I/O ports "
> > > 			"upon user request\n");
> > 
> > MBR, Sergei
> > 
> 
> Best regards,
> 
> Luiz Carlos Ramos

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 2/2] ide-generic: added messages in case of PCI IDE devices

[Sergei Shtylyov]

On 02/22/2017 03:38 AM, Luiz Carlos Ramos wrote:

>> On 02/21/2017 05:22 PM, Luiz Carlos Ramos wrote:
>>
>>> Two messages are now displayed (at "warn" level) to make sure the user
>>> is aware that PCI IDE devices were detected at common addresses (0x1f0
>>> and 0x170) and that they will not be managed by ide-generic, but rather
>>> delegated to its specific driver.
>>>
>>> Signed-off-by: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
>>> ---
>>> drivers/ide/ide-generic.c | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/ide/ide-generic.c b/drivers/ide/ide-generic.c
>>> index ee11edcdba3170c077381d603918498d79ffa3bb..2204819ef73612deed4ebfe58c69cdadb40fa53b 100644
>>> --- a/drivers/ide/ide-generic.c
>>> +++ b/drivers/ide/ide-generic.c
>>> @@ -125,9 +125,19 @@ static int __init ide_generic_init(void)
>>>
>>> 		if (primary == 0)
>>> 			probe_mask |= 0x1;
>>> +		else {
>>> +			printk(KERN_WARN DRV_NAME ": PCI IDE device detected at 0x%4.4x."
>>
>>    Also, please use pr_warn().
>>
>
> This code is quite old, and it is in deep maintenance phase.
>
> Every other message in ide-generic.c is implemented as printk(...),
> but to be fair, there are not so many of them.
>
> Given that fact, would it be better to change these two ones to be
> inserted to pr_warn() as suggested (and then having a mix of printk()
> and pr_warn() in the code), or to "allow" two more printk() in the code
> and having checkpatck.pl complaining?
>
> I really don't know what is better or worse. Both can be implemented,
> and there are a mix of advantages and disadvantages in each side.
>
> I'd like to hear from you and colleagues about how to proceed.

    I'd prefer a checkpatch-clean patch. Though I don't think DaveM would take 
further cleanups (printk -> pr_<foo>).

MBR, Sergei