Re: [PATCH 04/40] libata: move ata_{port,link,dev}_dbg to standard dev_XXX() macros

Re: [PATCH 04/40] libata: move ata_{port,link,dev}_dbg to standard dev_XXX() macros

[Geert Uytterhoeven]

 	Hi Hannes,

On Tue, 3 Mar 2020, Hannes Reinecke wrote:
> Use standard dev_{dbg,info,notice,warn,err} macros instead of the
> hand-crafted printk helpers.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

This is now commit ad9f23bd12e1d721 ("libata: move
ata_{port,link,dev}_dbg to standard dev_XXX() macros") in
linux-block/for-next.

This patch causes an intriguing change in boot messages, adding a space
at the beginning of each printed line:

      scsi host0: sata_rcar
     -ata1: SATA max UDMA/133 irq 117
     + ata1: SATA max UDMA/133 irq 117

     -ata1: link resume succeeded after 1 retries
     + link1: link resume succeeded after 1 retries

     -ata1: SATA link down (SStatus 0 SControl 300)
     + link1: SATA link down (SStatus 0 SControl 300)

It turns out dev_driver_string(&link->tdev) returns an empty string, as
its driver field is NULL, so __dev_printk() prints the empty string and
the device name, separated by a space.

At first I thought this was a bug in rcar-sata, lacking some setup that
was harmless before, but it turns out other drivers (e.g. pata-falcon)
show the same issue:

      scsi host0: pata_falcon
     -ata1: PATA max PIO4 cmd 0xfff00000 ctl 0xfff00039
     + ata1: PATA max PIO4 cmd 0xfff00000 ctl 0xfff00039

     -ata1.01: NODEV after polling detection
     -ata1.00: ATA-2: Sarge m68k, , max PIO2
     -ata1.00: 2118816 sectors, multi 0: LBA
     -ata1.00: configured for PIO
     + dev1.0: ATA-2: Sarge m68k, , max PIO2
     + dev1.0: 2118816 sectors, multi 0: LBA
     + dev1.0: configured for PIO

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

Re: [PATCH 04/40] libata: move ata_{port,link,dev}_dbg to standard dev_XXX() macros

[Bartlomiej Zolnierkiewicz]

On 3/24/20 2:26 PM, Geert Uytterhoeven wrote:
>     Hi Hannes,
> 
> On Tue, 3 Mar 2020, Hannes Reinecke wrote:
>> Use standard dev_{dbg,info,notice,warn,err} macros instead of the
>> hand-crafted printk helpers.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> 
> This is now commit ad9f23bd12e1d721 ("libata: move
> ata_{port,link,dev}_dbg to standard dev_XXX() macros") in
> linux-block/for-next.
> 
> This patch causes an intriguing change in boot messages, adding a space
> at the beginning of each printed line:
> 
>      scsi host0: sata_rcar
>     -ata1: SATA max UDMA/133 irq 117
>     + ata1: SATA max UDMA/133 irq 117
> 
>     -ata1: link resume succeeded after 1 retries
>     + link1: link resume succeeded after 1 retries
> 
>     -ata1: SATA link down (SStatus 0 SControl 300)
>     + link1: SATA link down (SStatus 0 SControl 300)
> 
> It turns out dev_driver_string(&link->tdev) returns an empty string, as
> its driver field is NULL, so __dev_printk() prints the empty string and
> the device name, separated by a space.
> 
> At first I thought this was a bug in rcar-sata, lacking some setup that
> was harmless before, but it turns out other drivers (e.g. pata-falcon)
> show the same issue:
> 
>      scsi host0: pata_falcon
>     -ata1: PATA max PIO4 cmd 0xfff00000 ctl 0xfff00039
>     + ata1: PATA max PIO4 cmd 0xfff00000 ctl 0xfff00039
> 
>     -ata1.01: NODEV after polling detection
>     -ata1.00: ATA-2: Sarge m68k, , max PIO2
>     -ata1.00: 2118816 sectors, multi 0: LBA
>     -ata1.00: configured for PIO
>     + dev1.0: ATA-2: Sarge m68k, , max PIO2
>     + dev1.0: 2118816 sectors, multi 0: LBA
>     + dev1.0: configured for PIO

I'm more worried about change of ATA devices and ATA links names
(unfortunately we cannot change the ones used in libata-transport.c 
as they are a part of ABI exposed through sysfs).

One way to improve the situation is to use transport classes names
for dev_printk() when no other means are available, please see/try
the patch below (compile tested only). It also includes fixup for
extra space issue (change to __dev_printk()).

We should probably consider reverting the commit if the approach
with using transport classes names is not feasible (we should not be
confusing users with names like "dev1.0" instead of "ata1.00" etc.,
sorry for not catching this earlier).

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


---
 drivers/ata/libata-transport.c |    3 +++
 drivers/base/core.c            |   14 +++++++++-----
 include/linux/device.h         |    7 ++++++-
 3 files changed, 18 insertions(+), 6 deletions(-)

Index: b/drivers/ata/libata-transport.c
===================================================================
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -289,6 +289,7 @@ int ata_tport_add(struct device *parent,
 	ata_host_get(ap->host);
 	dev->release = ata_tport_release;
 	dev_set_name(dev, "ata%d", ap->print_id);
+	dev->tclass = &ata_port_class;
 	transport_setup_device(dev);
 	ata_acpi_bind_port(ap);
 	error = device_add(dev);
@@ -418,6 +419,7 @@ int ata_tlink_add(struct ata_link *link)
 		dev_set_name(dev, "link%d", ap->print_id);
 	else
 		dev_set_name(dev, "link%d.%d", ap->print_id, link->pmp);
+	dev->tclass = &ata_link_class;
 
 	transport_setup_device(dev);
 
@@ -669,6 +671,7 @@ static int ata_tdev_add(struct ata_devic
 		dev_set_name(dev, "dev%d.%d", ap->print_id,ata_dev->devno);
 	else
 		dev_set_name(dev, "dev%d.%d.0", ap->print_id, link->pmp);
+	dev->tclass = &ata_dev_class;
 
 	transport_setup_device(dev);
 	ata_acpi_bind_dev(ata_dev);
Index: b/drivers/base/core.c
===================================================================
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1225,7 +1225,8 @@ const char *dev_driver_string(const stru
 	drv = READ_ONCE(dev->driver);
 	return drv ? drv->name :
 			(dev->bus ? dev->bus->name :
-			(dev->class ? dev->class->name : ""));
+			 (dev->class ? dev->class->name :
+			  (dev->tclass ? (&dev->tclass->class)->name : "")));
 }
 EXPORT_SYMBOL(dev_driver_string);
 
@@ -3784,10 +3785,13 @@ EXPORT_SYMBOL(dev_printk_emit);
 static void __dev_printk(const char *level, const struct device *dev,
 			struct va_format *vaf)
 {
-	if (dev)
-		dev_printk_emit(level[1] - '0', dev, "%s %s: %pV",
-				dev_driver_string(dev), dev_name(dev), vaf);
-	else
+	if (dev) {
+		const char *drv_str = dev_driver_string(dev);
+
+		dev_printk_emit(level[1] - '0', dev, "%s%s%s: %pV",
+				drv_str, strcmp(drv_str, "") ? " " : "",
+				dev_name(dev), vaf);
+	} else
 		printk("%s(NULL device *): %pV", level, vaf);
 }
 
Index: b/include/linux/device.h
===================================================================
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -29,6 +29,7 @@
 #include <linux/device/bus.h>
 #include <linux/device/class.h>
 #include <linux/device/driver.h>
+#include <linux/transport_class.h>
 #include <asm/device.h>
 
 struct device;
@@ -608,7 +609,11 @@ struct device {
 	spinlock_t		devres_lock;
 	struct list_head	devres_head;
 
-	struct class		*class;
+	union {
+		struct class		*class;
+		struct transport_class	*tclass;
+	};
+
 	const struct attribute_group **groups;	/* optional groups */
 
 	void	(*release)(struct device *dev);

Re: [PATCH 04/40] libata: move ata_{port,link,dev}_dbg to standard dev_XXX() macros

[Geert Uytterhoeven]

Hi Bartl,

On Wed, Mar 25, 2020 at 3:56 PM Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
> On 3/24/20 2:26 PM, Geert Uytterhoeven wrote:
> > On Tue, 3 Mar 2020, Hannes Reinecke wrote:
> >> Use standard dev_{dbg,info,notice,warn,err} macros instead of the
> >> hand-crafted printk helpers.
> >>
> >> Signed-off-by: Hannes Reinecke <hare@suse.de>
> >> Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> >
> > This is now commit ad9f23bd12e1d721 ("libata: move
> > ata_{port,link,dev}_dbg to standard dev_XXX() macros") in
> > linux-block/for-next.
> >
> > This patch causes an intriguing change in boot messages, adding a space
> > at the beginning of each printed line:
> >
> >      scsi host0: sata_rcar
> >     -ata1: SATA max UDMA/133 irq 117
> >     + ata1: SATA max UDMA/133 irq 117
> >
> >     -ata1: link resume succeeded after 1 retries
> >     + link1: link resume succeeded after 1 retries
> >
> >     -ata1: SATA link down (SStatus 0 SControl 300)
> >     + link1: SATA link down (SStatus 0 SControl 300)
> >
> > It turns out dev_driver_string(&link->tdev) returns an empty string, as
> > its driver field is NULL, so __dev_printk() prints the empty string and
> > the device name, separated by a space.
> >
> > At first I thought this was a bug in rcar-sata, lacking some setup that
> > was harmless before, but it turns out other drivers (e.g. pata-falcon)
> > show the same issue:
> >
> >      scsi host0: pata_falcon
> >     -ata1: PATA max PIO4 cmd 0xfff00000 ctl 0xfff00039
> >     + ata1: PATA max PIO4 cmd 0xfff00000 ctl 0xfff00039
> >
> >     -ata1.01: NODEV after polling detection
> >     -ata1.00: ATA-2: Sarge m68k, , max PIO2
> >     -ata1.00: 2118816 sectors, multi 0: LBA
> >     -ata1.00: configured for PIO
> >     + dev1.0: ATA-2: Sarge m68k, , max PIO2
> >     + dev1.0: 2118816 sectors, multi 0: LBA
> >     + dev1.0: configured for PIO
>
> I'm more worried about change of ATA devices and ATA links names
> (unfortunately we cannot change the ones used in libata-transport.c
> as they are a part of ABI exposed through sysfs).

True.

> One way to improve the situation is to use transport classes names
> for dev_printk() when no other means are available, please see/try
> the patch below (compile tested only). It also includes fixup for
> extra space issue (change to __dev_printk()).

Thanks for the suggestion!

> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -1225,7 +1225,8 @@ const char *dev_driver_string(const stru
>         drv = READ_ONCE(dev->driver);
>         return drv ? drv->name :
>                         (dev->bus ? dev->bus->name :
> -                       (dev->class ? dev->class->name : ""));
> +                        (dev->class ? dev->class->name :
> +                         (dev->tclass ? (&dev->tclass->class)->name : "")));

Aren't "dev->class->name" and "(&dev->tclass->class)->name"
exactly the same, as the first member of struct transport_class is
a struct class object?

(If they're not the same, and I need more coffee, I'm still afraid this is
 too fragile.  Then there is no guarantee the pointer points to a real
transport_class object, so this might cause a crash.)

> --- a/drivers/ata/libata-transport.c
> +++ b/drivers/ata/libata-transport.c
> @@ -289,6 +289,7 @@ int ata_tport_add(struct device *parent,
>         ata_host_get(ap->host);
>         dev->release = ata_tport_release;
>         dev_set_name(dev, "ata%d", ap->print_id);
> +       dev->tclass = &ata_port_class;

Can't you just do dev->class = &ata_port_class.class...

>         transport_setup_device(dev);
>         ata_acpi_bind_port(ap);
>         error = device_add(dev);
> @@ -418,6 +419,7 @@ int ata_tlink_add(struct ata_link *link)
>                 dev_set_name(dev, "link%d", ap->print_id);
>         else
>                 dev_set_name(dev, "link%d.%d", ap->print_id, link->pmp);
> +       dev->tclass = &ata_link_class;

... and dev->class = &ata_link_class.class...

>
>         transport_setup_device(dev);
>
> @@ -669,6 +671,7 @@ static int ata_tdev_add(struct ata_devic
>                 dev_set_name(dev, "dev%d.%d", ap->print_id,ata_dev->devno);
>         else
>                 dev_set_name(dev, "dev%d.%d.0", ap->print_id, link->pmp);
> +       dev->tclass = &ata_dev_class;

... and dev->class = &ata_dev_class.class...

> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -29,6 +29,7 @@
>  #include <linux/device/bus.h>
>  #include <linux/device/class.h>
>  #include <linux/device/driver.h>
> +#include <linux/transport_class.h>
>  #include <asm/device.h>
>
>  struct device;
> @@ -608,7 +609,11 @@ struct device {
>         spinlock_t              devres_lock;
>         struct list_head        devres_head;
>
> -       struct class            *class;
> +       union {
> +               struct class            *class;
> +               struct transport_class  *tclass;
> +       };
> +
>         const struct attribute_group **groups;  /* optional groups */
>
>         void    (*release)(struct device *dev);

... and drop all changes to device.h?

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

Re: [PATCH 04/40] libata: move ata_{port,link,dev}_dbg to standard dev_XXX() macros

[Bartlomiej Zolnierkiewicz]

On 3/25/20 4:34 PM, Geert Uytterhoeven wrote:
> Hi Bartl,
> 
> On Wed, Mar 25, 2020 at 3:56 PM Bartlomiej Zolnierkiewicz
> <b.zolnierkie@samsung.com> wrote:
>> On 3/24/20 2:26 PM, Geert Uytterhoeven wrote:
>>> On Tue, 3 Mar 2020, Hannes Reinecke wrote:
>>>> Use standard dev_{dbg,info,notice,warn,err} macros instead of the
>>>> hand-crafted printk helpers.
>>>>
>>>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>>>> Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>>>
>>> This is now commit ad9f23bd12e1d721 ("libata: move
>>> ata_{port,link,dev}_dbg to standard dev_XXX() macros") in
>>> linux-block/for-next.
>>>
>>> This patch causes an intriguing change in boot messages, adding a space
>>> at the beginning of each printed line:
>>>
>>>      scsi host0: sata_rcar
>>>     -ata1: SATA max UDMA/133 irq 117
>>>     + ata1: SATA max UDMA/133 irq 117
>>>
>>>     -ata1: link resume succeeded after 1 retries
>>>     + link1: link resume succeeded after 1 retries
>>>
>>>     -ata1: SATA link down (SStatus 0 SControl 300)
>>>     + link1: SATA link down (SStatus 0 SControl 300)
>>>
>>> It turns out dev_driver_string(&link->tdev) returns an empty string, as
>>> its driver field is NULL, so __dev_printk() prints the empty string and
>>> the device name, separated by a space.
>>>
>>> At first I thought this was a bug in rcar-sata, lacking some setup that
>>> was harmless before, but it turns out other drivers (e.g. pata-falcon)
>>> show the same issue:
>>>
>>>      scsi host0: pata_falcon
>>>     -ata1: PATA max PIO4 cmd 0xfff00000 ctl 0xfff00039
>>>     + ata1: PATA max PIO4 cmd 0xfff00000 ctl 0xfff00039
>>>
>>>     -ata1.01: NODEV after polling detection
>>>     -ata1.00: ATA-2: Sarge m68k, , max PIO2
>>>     -ata1.00: 2118816 sectors, multi 0: LBA
>>>     -ata1.00: configured for PIO
>>>     + dev1.0: ATA-2: Sarge m68k, , max PIO2
>>>     + dev1.0: 2118816 sectors, multi 0: LBA
>>>     + dev1.0: configured for PIO
>>
>> I'm more worried about change of ATA devices and ATA links names
>> (unfortunately we cannot change the ones used in libata-transport.c
>> as they are a part of ABI exposed through sysfs).
> 
> True.
> 
>> One way to improve the situation is to use transport classes names
>> for dev_printk() when no other means are available, please see/try
>> the patch below (compile tested only). It also includes fixup for
>> extra space issue (change to __dev_printk()).
> 
> Thanks for the suggestion!
> 
>> --- a/drivers/base/core.c
>> +++ b/drivers/base/core.c
>> @@ -1225,7 +1225,8 @@ const char *dev_driver_string(const stru
>>         drv = READ_ONCE(dev->driver);
>>         return drv ? drv->name :
>>                         (dev->bus ? dev->bus->name :
>> -                       (dev->class ? dev->class->name : ""));
>> +                        (dev->class ? dev->class->name :
>> +                         (dev->tclass ? (&dev->tclass->class)->name : "")));
> 
> Aren't "dev->class->name" and "(&dev->tclass->class)->name"
> exactly the same, as the first member of struct transport_class is
> a struct class object?

struct class is embedded in struct transport_class but it doesn't
have to be the same class as pointed by dev->class (for ATA dev->class
should be NULL).

> (If they're not the same, and I need more coffee, I'm still afraid this is
>  too fragile.  Then there is no guarantee the pointer points to a real
> transport_class object, so this might cause a crash.)

I see, we can't do unnamed union optimization in <linux/device.h> but
otherwise it should be OK, no?

>> --- a/drivers/ata/libata-transport.c
>> +++ b/drivers/ata/libata-transport.c
>> @@ -289,6 +289,7 @@ int ata_tport_add(struct device *parent,
>>         ata_host_get(ap->host);
>>         dev->release = ata_tport_release;
>>         dev_set_name(dev, "ata%d", ap->print_id);
>> +       dev->tclass = &ata_port_class;
> 
> Can't you just do dev->class = &ata_port_class.class...
> 
>>         transport_setup_device(dev);
>>         ata_acpi_bind_port(ap);
>>         error = device_add(dev);
>> @@ -418,6 +419,7 @@ int ata_tlink_add(struct ata_link *link)
>>                 dev_set_name(dev, "link%d", ap->print_id);
>>         else
>>                 dev_set_name(dev, "link%d.%d", ap->print_id, link->pmp);
>> +       dev->tclass = &ata_link_class;
> 
> ... and dev->class = &ata_link_class.class...
> 
>>
>>         transport_setup_device(dev);
>>
>> @@ -669,6 +671,7 @@ static int ata_tdev_add(struct ata_devic
>>                 dev_set_name(dev, "dev%d.%d", ap->print_id,ata_dev->devno);
>>         else
>>                 dev_set_name(dev, "dev%d.%d.0", ap->print_id, link->pmp);
>> +       dev->tclass = &ata_dev_class;
> 
> ... and dev->class = &ata_dev_class.class...
> 
>> --- a/include/linux/device.h
>> +++ b/include/linux/device.h
>> @@ -29,6 +29,7 @@
>>  #include <linux/device/bus.h>
>>  #include <linux/device/class.h>
>>  #include <linux/device/driver.h>
>> +#include <linux/transport_class.h>
>>  #include <asm/device.h>
>>
>>  struct device;
>> @@ -608,7 +609,11 @@ struct device {
>>         spinlock_t              devres_lock;
>>         struct list_head        devres_head;
>>
>> -       struct class            *class;
>> +       union {
>> +               struct class            *class;
>> +               struct transport_class  *tclass;
>> +       };
>> +
>>         const struct attribute_group **groups;  /* optional groups */
>>
>>         void    (*release)(struct device *dev);
> 
> ... and drop all changes to device.h?

That would have unintended consequences that we don't want
(driver core code checks dev->class existence not only in
dev_driver_string())..

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

Re: [PATCH 04/40] libata: move ata_{port,link,dev}_dbg to standard dev_XXX() macros

[Hannes Reinecke]

On 3/25/20 4:45 PM, Bartlomiej Zolnierkiewicz wrote:
> 
> On 3/25/20 4:34 PM, Geert Uytterhoeven wrote:
>> Hi Bartl,
>>
>> On Wed, Mar 25, 2020 at 3:56 PM Bartlomiej Zolnierkiewicz
>> <b.zolnierkie@samsung.com> wrote:
>>> On 3/24/20 2:26 PM, Geert Uytterhoeven wrote:
>>>> On Tue, 3 Mar 2020, Hannes Reinecke wrote:
>>>>> Use standard dev_{dbg,info,notice,warn,err} macros instead of the
>>>>> hand-crafted printk helpers.
>>>>>
>>>>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>>>>> Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>>>>
>>>> This is now commit ad9f23bd12e1d721 ("libata: move
>>>> ata_{port,link,dev}_dbg to standard dev_XXX() macros") in
>>>> linux-block/for-next.
>>>>
>>>> This patch causes an intriguing change in boot messages, adding a space
>>>> at the beginning of each printed line:
>>>>
>>>>       scsi host0: sata_rcar
>>>>      -ata1: SATA max UDMA/133 irq 117
>>>>      + ata1: SATA max UDMA/133 irq 117
>>>>
>>>>      -ata1: link resume succeeded after 1 retries
>>>>      + link1: link resume succeeded after 1 retries
>>>>
>>>>      -ata1: SATA link down (SStatus 0 SControl 300)
>>>>      + link1: SATA link down (SStatus 0 SControl 300)
>>>>
>>>> It turns out dev_driver_string(&link->tdev) returns an empty string, as
>>>> its driver field is NULL, so __dev_printk() prints the empty string and
>>>> the device name, separated by a space.
>>>>
>>>> At first I thought this was a bug in rcar-sata, lacking some setup that
>>>> was harmless before, but it turns out other drivers (e.g. pata-falcon)
>>>> show the same issue:
>>>>
>>>>       scsi host0: pata_falcon
>>>>      -ata1: PATA max PIO4 cmd 0xfff00000 ctl 0xfff00039
>>>>      + ata1: PATA max PIO4 cmd 0xfff00000 ctl 0xfff00039
>>>>
>>>>      -ata1.01: NODEV after polling detection
>>>>      -ata1.00: ATA-2: Sarge m68k, , max PIO2
>>>>      -ata1.00: 2118816 sectors, multi 0: LBA
>>>>      -ata1.00: configured for PIO
>>>>      + dev1.0: ATA-2: Sarge m68k, , max PIO2
>>>>      + dev1.0: 2118816 sectors, multi 0: LBA
>>>>      + dev1.0: configured for PIO
>>>
>>> I'm more worried about change of ATA devices and ATA links names
>>> (unfortunately we cannot change the ones used in libata-transport.c
>>> as they are a part of ABI exposed through sysfs).
>>
>> True.
>>
>>> One way to improve the situation is to use transport classes names
>>> for dev_printk() when no other means are available, please see/try
>>> the patch below (compile tested only). It also includes fixup for
>>> extra space issue (change to __dev_printk()).
>>
>> Thanks for the suggestion!
>>
Well, the space can (should?) be fixed with a simple patch to 
__dev_printk() but just checking if 'dev_driver_string()' returns a 
value or not:

diff --git a/drivers/base/core.c b/drivers/base/core.c
index dbb0f9130f42..a84f170a1a1f 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -3635,10 +3635,15 @@ EXPORT_SYMBOL(dev_printk_emit);
  static void __dev_printk(const char *level, const struct device *dev,
                         struct va_format *vaf)
  {
-       if (dev)
-               dev_printk_emit(level[1] - '0', dev, "%s %s: %pV",
-                               dev_driver_string(dev), dev_name(dev), vaf);
-       else
+       if (dev) {
+               if (dev_string_string(dev))
+                       dev_printk_emit(level[1] - '0', dev, "%s %s: %pV",
+                                       dev_driver_string(dev), 
dev_name(dev),
+                                       vaf);
+               else
+                       dev_printk_emit(level[1] - '0', dev, "%s: %pV",
+                                       dev_name(dev), vaf);
+       } else
                 printk("%s(NULL device *): %pV", level, vaf);
  }

which actually makes sense on its own.
As for the name change, yes, I'm not that happy with it, either.

I'll see if we can do some transport class magic such that the
names do not change.

(And no-one is to mention 'cake' and 'eat it' ...)

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare@suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer