Patchwork ata_piix: minor typo fixes and threading fix

login
register
mail settings
Submitter Levente Kurusa
Date Sept. 22, 2013, 1:53 p.m.
Message ID <523EF645.7050808@linux.com>
Download mbox | patch
Permalink /patch/276996/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Levente Kurusa - Sept. 22, 2013, 1:53 p.m.
Hi,

The following patch fixes a printk() call, which was originally used 
with pr_cont, which will however fail. Fixed that with setting up a 
buffer to save data to that first, and then printk() it. The patch also
fixes some minor typos and a comment, so that it better reflects the
documentation of ICH*.

Regards,
Levente Kurusa

Signed-off-by: Levente Kurusa <levex@linux.com>
---
  	if (invalid_map)
  		dev_err(&pdev->dev, "invalid MAP value %u\n", map_value);
@@ -1718,18 +1741,17 @@ static int piix_init_one(struct pci_dev *pdev, 
const struct pci_device_id *ent)
  		if (host->ports[0]->ops == &piix_sidpr_sata_ops)
  			sht = &piix_sidpr_sht;
  	}
-
  	/* apply IOCFG bit18 quirk */
  	piix_iocfg_bit18_quirk(host);

-	/* On ICH5, some BIOSen disable the interrupt using the
+	/* On ICH5, some BIOSes disable the interrupt using the
  	 * PCI_COMMAND_INTX_DISABLE bit added in PCI 2.3.
  	 * On ICH6, this bit has the same effect, but only when
  	 * MSI is disabled (and it is disabled, as we don't use
  	 * message-signalled interrupts currently).
  	 */
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov - Sept. 22, 2013, 2:44 p.m.
Hello.

On 22.09.2013 17:53, Levente Kurusa wrote:

> Hi,

> The following patch fixes a printk() call, which was originally used with
> pr_cont, which will however fail. Fixed that with setting up a buffer to save
> data to that first, and then printk() it. The patch also
> fixes some minor typos and a comment, so that it better reflects the
> documentation of ICH*.

    Wrap your changelog lines at 80 columns (preferably less).

> Regards,
> Levente Kurusa

    The patch changelog is not a place for greetings and regards.

> Signed-off-by: Levente Kurusa <levex@linux.com>
> ---
> diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
> index 93cb092..b7bf3df 100644
> --- a/drivers/ata/ata_piix.c
> +++ b/drivers/ata/ata_piix.c
> @@ -100,7 +100,7 @@
>
>   enum {

    Your patch is whitespace damaged, there was no space on previous line, and 
there are two spaces starting this line (instead of one).

> @@ -1368,34 +1368,53 @@ static const int *piix_init_sata_map(struct pci_dev
> *pdev,
>       pci_read_config_byte(pdev, ICH5_PMR, &map_value);
>
>       map = map_db->map[map_value & map_db->mask];
> -
> -    dev_info(&pdev->dev, "MAP [");
> +    char* mapdata[4];

    Don't mix declarations and data.

>       for (i = 0; i < 4; i++) {
>           switch (map[i]) {
>           case RV:
>               invalid_map = 1;
> -            pr_cont(" XX");
> +            mapdata[i] =" XX";
>               break;
>
>           case NA:
> -            pr_cont(" --");
> +            mapdata[i] = " --";
>               break;
>
>           case IDE:
>               WARN_ON((i & 1) || map[i + 1] != IDE);
>               pinfo[i / 2] = piix_port_info[ich_pata_100];
>               i++;
> -            pr_cont(" IDE IDE");
> +            mapdata[i] = " IDE IDE";
> +            break;
> +        case P0:
> +            mapdata[i] = " P0";
> +            if (i & 1)
> +                pinfo[i / 2].flags |= ATA_FLAG_SLAVE_POSS;
> +            break;
> +        case P1:
> +            mapdata[i] = " P1";
> +            if (i & 1)
> +                pinfo[i / 2].flags |= ATA_FLAG_SLAVE_POSS;
> +            break;
> +        case P2:
> +            mapdata[i] = " P2";
> +            if (i & 1)
> +                pinfo[i / 2].flags |= ATA_FLAG_SLAVE_POSS;
> +            break;
> +        case P3:
> +            mapdata[i] = " P3";
> +            pinfo[i / 2].flags |= ATA_FLAG_SLAVE_POSS;
> +            if (i & 1)
> +                pinfo[i / 2].flags |= ATA_FLAG_SLAVE_POSS;
>               break;

    I don't think the code repeating 4 times is a good idea.

> -
>           default:
> -            pr_cont(" P%d", map[i]);
> +            mapdata[i] = " INV";
>               if (i & 1)
>                   pinfo[i / 2].flags |= ATA_FLAG_SLAVE_POSS;
>               break;
>           }
>       }
> -    pr_cont(" ]\n");
> +    dev_info(&pdev->dev, "MAP [%s%s%s%s ]\n", mapdata[0], mapdata[1],
> mapdata[2], mapdata[3], map_value, map_db->mask);

    You didn't specify the formats for the last 2 arguments and your line is 
longer than 80 columns, please wrap it.

> @@ -1718,18 +1741,17 @@ static int piix_init_one(struct pci_dev *pdev, const
> struct pci_device_id *ent)
>           if (host->ports[0]->ops == &piix_sidpr_sata_ops)
>               sht = &piix_sidpr_sht;
>       }
> -

    Why?

>       /* apply IOCFG bit18 quirk */
>       piix_iocfg_bit18_quirk(host);
>
> -    /* On ICH5, some BIOSen disable the interrupt using the
> +    /* On ICH5, some BIOSes disable the interrupt using the

    Why? "BIOSen" is not a typo, it's a jargon.

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Levente Kurusa - Sept. 22, 2013, 3:05 p.m.
2013-09-22 16:44 keltezéssel, Sergei Shtylyov írta:
> Hello.
>
> On 22.09.2013 17:53, Levente Kurusa wrote:
>
>> Hi,
>
>> The following patch fixes a printk() call, which was originally used with
>> pr_cont, which will however fail. Fixed that with setting up a buffer
>> to save
>> data to that first, and then printk() it. The patch also
>> fixes some minor typos and a comment, so that it better reflects the
>> documentation of ICH*.
>
>     Wrap your changelog lines at 80 columns (preferably less).
>
>> Regards,
>> Levente Kurusa
>
>     The patch changelog is not a place for greetings and regards.
Sorry for that.
>
>> Signed-off-by: Levente Kurusa <levex@linux.com>
>> ---
>> diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
>> index 93cb092..b7bf3df 100644
>> --- a/drivers/ata/ata_piix.c
>> +++ b/drivers/ata/ata_piix.c
>> @@ -100,7 +100,7 @@
>>
>>   enum {
>
>     Your patch is whitespace damaged, there was no space on previous
> line, and there are two spaces starting this line (instead of one).
>
>> @@ -1368,34 +1368,53 @@ static const int *piix_init_sata_map(struct
>> pci_dev
>> *pdev,
>>       pci_read_config_byte(pdev, ICH5_PMR, &map_value);
>>
>>       map = map_db->map[map_value & map_db->mask];
>> -
>> -    dev_info(&pdev->dev, "MAP [");
>> +    char* mapdata[4];
>
>     Don't mix declarations and data.
>
>>       for (i = 0; i < 4; i++) {
>>           switch (map[i]) {
>>           case RV:
>>               invalid_map = 1;
>> -            pr_cont(" XX");
>> +            mapdata[i] =" XX";
>>               break;
>>
>>           case NA:
>> -            pr_cont(" --");
>> +            mapdata[i] = " --";
>>               break;
>>
>>           case IDE:
>>               WARN_ON((i & 1) || map[i + 1] != IDE);
>>               pinfo[i / 2] = piix_port_info[ich_pata_100];
>>               i++;
>> -            pr_cont(" IDE IDE");
>> +            mapdata[i] = " IDE IDE";
>> +            break;
>> +        case P0:
>> +            mapdata[i] = " P0";
>> +            if (i & 1)
>> +                pinfo[i / 2].flags |= ATA_FLAG_SLAVE_POSS;
>> +            break;
>> +        case P1:
>> +            mapdata[i] = " P1";
>> +            if (i & 1)
>> +                pinfo[i / 2].flags |= ATA_FLAG_SLAVE_POSS;
>> +            break;
>> +        case P2:
>> +            mapdata[i] = " P2";
>> +            if (i & 1)
>> +                pinfo[i / 2].flags |= ATA_FLAG_SLAVE_POSS;
>> +            break;
>> +        case P3:
>> +            mapdata[i] = " P3";
>> +            pinfo[i / 2].flags |= ATA_FLAG_SLAVE_POSS;
>> +            if (i & 1)
>> +                pinfo[i / 2].flags |= ATA_FLAG_SLAVE_POSS;
>>               break;
>
>     I don't think the code repeating 4 times is a good idea.
I did this to prevent a goto, but changing that back then.
>
>> -
>>           default:
>> -            pr_cont(" P%d", map[i]);
>> +            mapdata[i] = " INV";
>>               if (i & 1)
>>                   pinfo[i / 2].flags |= ATA_FLAG_SLAVE_POSS;
>>               break;
>>           }
>>       }
>> -    pr_cont(" ]\n");
>> +    dev_info(&pdev->dev, "MAP [%s%s%s%s ]\n", mapdata[0], mapdata[1],
>> mapdata[2], mapdata[3], map_value, map_db->mask);
That was from my former debug mechanism, overlooked that. Sorry.
>
>     You didn't specify the formats for the last 2 arguments and your
> line is longer than 80 columns, please wrap it.
>
>> @@ -1718,18 +1741,17 @@ static int piix_init_one(struct pci_dev *pdev,
>> const
>> struct pci_device_id *ent)
>>           if (host->ports[0]->ops == &piix_sidpr_sata_ops)
>>               sht = &piix_sidpr_sht;
>>       }
>> -
>
>     Why?
>
>>       /* apply IOCFG bit18 quirk */
>>       piix_iocfg_bit18_quirk(host);
>>
>> -    /* On ICH5, some BIOSen disable the interrupt using the
>> +    /* On ICH5, some BIOSes disable the interrupt using the
>
>     Why? "BIOSen" is not a typo, it's a jargon.
>
> WBR, Sergei
>
>
Thank you for all your comments, I am not
a native writer of english, and hence I didn't have knowledge
of that jargon. I am fixing the issues, as per checkpatch.pl and
your guidelines. Once finished, I will repost it.
Sergei Shtylyov - Sept. 22, 2013, 3:10 p.m.
On 22-09-2013 18:44, Sergei Shtylyov wrote:

>> Hi,

>> The following patch fixes a printk() call, which was originally used with
>> pr_cont, which will however fail. Fixed that with setting up a buffer to save
>> data to that first, and then printk() it. The patch also
>> fixes some minor typos and a comment, so that it better reflects the
>> documentation of ICH*.

>     Wrap your changelog lines at 80 columns (preferably less).

>> Regards,
>> Levente Kurusa

>     The patch changelog is not a place for greetings and regards.

>> Signed-off-by: Levente Kurusa <levex@linux.com>
>> ---
>> diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
>> index 93cb092..b7bf3df 100644
>> --- a/drivers/ata/ata_piix.c
>> +++ b/drivers/ata/ata_piix.c
[...]
>> @@ -1368,34 +1368,53 @@ static const int *piix_init_sata_map(struct pci_dev
>> *pdev,
>>       pci_read_config_byte(pdev, ICH5_PMR, &map_value);
>>
>>       map = map_db->map[map_value & map_db->mask];
>> -
>> -    dev_info(&pdev->dev, "MAP [");
>> +    char* mapdata[4];

>     Don't mix declarations and data.

    Oops, I meant to type "code" instead of "data", of course.

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo - Sept. 22, 2013, 3:39 p.m.
On Sun, Sep 22, 2013 at 03:53:09PM +0200, Levente Kurusa wrote:
> Hi,
> 
> The following patch fixes a printk() call, which was originally used
> with pr_cont, which will however fail. Fixed that with setting up a

Why would pr_cont() fail?

Thanks.
Levente Kurusa - Sept. 22, 2013, 3:46 p.m.
On 2013-09-22 17:39, Tejun Heo wrote:
> On Sun, Sep 22, 2013 at 03:53:09PM +0200, Levente Kurusa wrote:
>> Hi,
>>
>> The following patch fixes a printk() call, which was originally used
>> with pr_cont, which will however fail. Fixed that with setting up a
> Why would pr_cont() fail?
>
> Thanks.
>
As far as I know pr_cont is not thread-safe. Without this patch I was 
only getting "... MAP [" and the rest would be scattered through the dmesg.


---
Regards,
Levente Kurusa
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo - Sept. 22, 2013, 4:05 p.m.
On Sun, Sep 22, 2013 at 05:46:03PM +0200, Levente Kurusa wrote:
> As far as I know pr_cont is not thread-safe. Without this patch I
> was only getting "... MAP [" and the rest would be scattered through
> the dmesg.

Continuations are now properly handled by printk.  Please see commits
leading to eab072609e11a357181806ab5a5c309ef6eb76f5.

Thanks.
Levente Kurusa - Sept. 25, 2013, 2:40 p.m.
I am getting the following output:
[    2.236379] ata_piix 0000:00:1f.2: MAP [
[    2.236492]  P0 P2 -- -- ]
Instead of the expected:
[    2.236379] ata_piix 0000:00:1f.2: MAP [ P0 P2 -- -- ]

This might be a minor problem, but I think it is better the second way.
Of course, now that I better understand the code, I see that my fix to this,
is not the best. I will make a new patch, if you think this should be fixed.

Regards,
Levente Kurusa

2013/9/22 Tejun Heo <tj@kernel.org>:
> On Sun, Sep 22, 2013 at 05:46:03PM +0200, Levente Kurusa wrote:
>> As far as I know pr_cont is not thread-safe. Without this patch I
>> was only getting "... MAP [" and the rest would be scattered through
>> the dmesg.
>
> Continuations are now properly handled by printk.  Please see commits
> leading to eab072609e11a357181806ab5a5c309ef6eb76f5.
>
> Thanks.
>
> --
> tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo - Sept. 26, 2013, 2:09 p.m.
(cc'ing Kay)

On Wed, Sep 25, 2013 at 04:40:30PM +0200, Levente Kurusa wrote:
> I am getting the following output:
> [    2.236379] ata_piix 0000:00:1f.2: MAP [
> [    2.236492]  P0 P2 -- -- ]
> Instead of the expected:
> [    2.236379] ata_piix 0000:00:1f.2: MAP [ P0 P2 -- -- ]
> 
> This might be a minor problem, but I think it is better the second way.
> Of course, now that I better understand the code, I see that my fix to this,
> is not the best. I will make a new patch, if you think this should be fixed.

Hmmm... so, you're saying pr_cont() is misbehaving after dev_info()?
If so, we don't want to paper over that from a low level driver like
ata_piix.  Let's find out what's going on with pr_cont().

Kay, Levente is seeing the above output from
drivers/ata/ata_piix.c::piix_init_sata_map().  Any ideas?

Thanks.
Levente Kurusa - Sept. 26, 2013, 5:14 p.m.
2013/9/26 Tejun Heo <tj@kernel.org>:
> (cc'ing Kay)
>
> On Wed, Sep 25, 2013 at 04:40:30PM +0200, Levente Kurusa wrote:
>> I am getting the following output:
>> [    2.236379] ata_piix 0000:00:1f.2: MAP [
>> [    2.236492]  P0 P2 -- -- ]
>> Instead of the expected:
>> [    2.236379] ata_piix 0000:00:1f.2: MAP [ P0 P2 -- -- ]
>>
>> This might be a minor problem, but I think it is better the second way.
>> Of course, now that I better understand the code, I see that my fix to this,
>> is not the best. I will make a new patch, if you think this should be fixed.
>
> Hmmm... so, you're saying pr_cont() is misbehaving after dev_info()?
> If so, we don't want to paper over that from a low level driver like
> ata_piix.  Let's find out what's going on with pr_cont().
>
Yes, however I am not familiar with printk() and the likes (like _dev_info()),
but it looks like either _dev_info() is putting a trailing '\n' or
only KERN_CONT
lines can be continued, which is probably not the way it is, because I think
that sort of defeats the purpose.

Regards,
Levente Kurusa
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kay Sievers - Sept. 26, 2013, 5:57 p.m.
On Thu, Sep 26, 2013 at 4:09 PM, Tejun Heo <tj@kernel.org> wrote:
> (cc'ing Kay)
>
> On Wed, Sep 25, 2013 at 04:40:30PM +0200, Levente Kurusa wrote:
>> I am getting the following output:
>> [    2.236379] ata_piix 0000:00:1f.2: MAP [
>> [    2.236492]  P0 P2 -- -- ]
>> Instead of the expected:
>> [    2.236379] ata_piix 0000:00:1f.2: MAP [ P0 P2 -- -- ]
>>
>> This might be a minor problem, but I think it is better the second way.
>> Of course, now that I better understand the code, I see that my fix to this,
>> is not the best. I will make a new patch, if you think this should be fixed.
>
> Hmmm... so, you're saying pr_cont() is misbehaving after dev_info()?
> If so, we don't want to paper over that from a low level driver like
> ata_piix.  Let's find out what's going on with pr_cont().
>
> Kay, Levente is seeing the above output from
> drivers/ata/ata_piix.c::piix_init_sata_map().  Any ideas?

Yes, the dev_printk() versions are "atomic lines", cannot be
concatenated with other invocations of printk() like the normal
printks can. The reason is that the dev_* versions carry structured
data to userspace, and there, the logs are indexed by device.

We don't want to allow the convenient but very racy and
non-thread-safe concatenation lines for dev_* versions, they might
pick up unrelated text fragments from other threads into the logged
text line; userspace (non-human tools) can not really cope with that.

So, the printk should not use the dev_* version, or print all in one
call, if a single line is expected.

Kay
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo - Sept. 27, 2013, 1:28 p.m.
Hello,

On Thu, Sep 26, 2013 at 07:57:35PM +0200, Kay Sievers wrote:
> > Kay, Levente is seeing the above output from
> > drivers/ata/ata_piix.c::piix_init_sata_map().  Any ideas?
> 
> Yes, the dev_printk() versions are "atomic lines", cannot be
> concatenated with other invocations of printk() like the normal
> printks can. The reason is that the dev_* versions carry structured
> data to userspace, and there, the logs are indexed by device.
> 
> We don't want to allow the convenient but very racy and
> non-thread-safe concatenation lines for dev_* versions, they might
> pick up unrelated text fragments from other threads into the logged
> text line; userspace (non-human tools) can not really cope with that.
>
> So, the printk should not use the dev_* version, or print all in one
> call, if a single line is expected.

I see.  Levente, can you please go ahead and update the patch?

Thanks.
Levente Kurusa - Sept. 27, 2013, 1:29 p.m.
2013-09-26 19:57 keltezéssel, Kay Sievers írta:
> On Thu, Sep 26, 2013 at 4:09 PM, Tejun Heo <tj@kernel.org> wrote:
>> (cc'ing Kay)
>>
>> On Wed, Sep 25, 2013 at 04:40:30PM +0200, Levente Kurusa wrote:
>>> I am getting the following output:
>>> [    2.236379] ata_piix 0000:00:1f.2: MAP [
>>> [    2.236492]  P0 P2 -- -- ]
>>> Instead of the expected:
>>> [    2.236379] ata_piix 0000:00:1f.2: MAP [ P0 P2 -- -- ]
>>>
>>> This might be a minor problem, but I think it is better the second way.
>>> Of course, now that I better understand the code, I see that my fix to this,
>>> is not the best. I will make a new patch, if you think this should be fixed.
>>
>> Hmmm... so, you're saying pr_cont() is misbehaving after dev_info()?
>> If so, we don't want to paper over that from a low level driver like
>> ata_piix.  Let's find out what's going on with pr_cont().
>>
>> Kay, Levente is seeing the above output from
>> drivers/ata/ata_piix.c::piix_init_sata_map().  Any ideas?
>
> Yes, the dev_printk() versions are "atomic lines", cannot be
> concatenated with other invocations of printk() like the normal
> printks can. The reason is that the dev_* versions carry structured
> data to userspace, and there, the logs are indexed by device.
>
> We don't want to allow the convenient but very racy and
> non-thread-safe concatenation lines for dev_* versions, they might
> pick up unrelated text fragments from other threads into the logged
> text line; userspace (non-human tools) can not really cope with that.
>
> So, the printk should not use the dev_* version, or print all in one
> call, if a single line is expected.
>
> Kay
>

Thank you Kay for clarifying this! I think I will go with
creating a buffer, and printing this all in one line.

Tejun, during this weekend I will create a patch that
hopefully fixes this. However, if you think that setting
up a temporary buffer is inappropriate, please comment.


Thank you,
Levente Kurusa
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 93cb092..b7bf3df 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -100,7 +100,7 @@ 

  enum {
  	PIIX_IOCFG		= 0x54, /* IDE I/O configuration register */
-	ICH5_PMR		= 0x90, /* port mapping register */
+	ICH5_PMR		= 0x90, /* address map register */
  	ICH5_PCS		= 0x92,	/* port control and status */
  	PIIX_SIDPR_BAR		= 5,
  	PIIX_SIDPR_LEN		= 16,
@@ -233,7 +233,7 @@  static const struct pci_device_id piix_pci_tbl[] = {
  	  PCI_CLASS_STORAGE_IDE << 8, 0xffff00, ich6m_sata },
  	/* 82801GB/GR/GH (ICH7, identical to ICH6) */
  	{ 0x8086, 0x27c0, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich6_sata },
-	/* 2801GBM/GHM (ICH7M, identical to ICH6M) */
+	/* 82801GBM/GHM (ICH7M, identical to ICH6M)  */
  	{ 0x8086, 0x27c4, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich6m_sata },
  	/* Enterprise Southbridge 2 (631xESB/632xESB) */
  	{ 0x8086, 0x2680, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich6_sata },
@@ -515,7 +515,7 @@  static int ich_pata_cable_detect(struct ata_port *ap)
  	const struct ich_laptop *lap = &ich_laptop[0];
  	u8 mask;

-	/* Check for specials - Acer Aspire 5602WLMi */
+	/* Check for specials */
  	while (lap->device) {
  		if (lap->device == pdev->device &&
  		    lap->subvendor == pdev->subsystem_vendor &&
@@ -1368,34 +1368,53 @@  static const int *piix_init_sata_map(struct 
pci_dev *pdev,
  	pci_read_config_byte(pdev, ICH5_PMR, &map_value);

  	map = map_db->map[map_value & map_db->mask];
-
-	dev_info(&pdev->dev, "MAP [");
+	char* mapdata[4];
  	for (i = 0; i < 4; i++) {
  		switch (map[i]) {
  		case RV:
  			invalid_map = 1;
-			pr_cont(" XX");
+			mapdata[i] =" XX";
  			break;

  		case NA:
-			pr_cont(" --");
+			mapdata[i] = " --";
  			break;

  		case IDE:
  			WARN_ON((i & 1) || map[i + 1] != IDE);
  			pinfo[i / 2] = piix_port_info[ich_pata_100];
  			i++;
-			pr_cont(" IDE IDE");
+			mapdata[i] = " IDE IDE";
+			break;
+		case P0:
+			mapdata[i] = " P0";
+			if (i & 1)
+				pinfo[i / 2].flags |= ATA_FLAG_SLAVE_POSS;
+			break;
+		case P1:
+			mapdata[i] = " P1";
+			if (i & 1)
+				pinfo[i / 2].flags |= ATA_FLAG_SLAVE_POSS;
+			break;
+		case P2:
+			mapdata[i] = " P2";
+			if (i & 1)
+				pinfo[i / 2].flags |= ATA_FLAG_SLAVE_POSS;
+			break;
+		case P3:
+			mapdata[i] = " P3";
+			pinfo[i / 2].flags |= ATA_FLAG_SLAVE_POSS;
+			if (i & 1)
+				pinfo[i / 2].flags |= ATA_FLAG_SLAVE_POSS;
  			break;
-
  		default:
-			pr_cont(" P%d", map[i]);
+			mapdata[i] = " INV";
  			if (i & 1)
  				pinfo[i / 2].flags |= ATA_FLAG_SLAVE_POSS;
  			break;
  		}
  	}
-	pr_cont(" ]\n");
+	dev_info(&pdev->dev, "MAP [%s%s%s%s ]\n", mapdata[0], mapdata[1], 
mapdata[2], mapdata[3], map_value, map_db->mask);