| Message ID | 20220103090939.36238-1-hare@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] libata: sysfs naming option | expand |
On 1/3/22 18:09, Hannes Reinecke wrote: > This patch adds a config option ATA_SYSFS_NAMING to align the libata > device names in sysfs with those in the kernel message log. > It adds a new dummy bus 'ata', which collects all ata device objects > like ata_port, ata_link, and ata_dev, and adds an 'ata' prefix to the > message log. > For consistency the 'ata_dev' name has been changed from 'ata' to 'dev'; > as this induces a sysfs change the config option is disabled per default. > > Patch is relative to the 'for-5.17-logging' branch from Damien. > > Signed-off-by: Hannes Reinecke <hare@suse.de> > --- > drivers/ata/Kconfig | 17 ++++++++++++++++ > drivers/ata/libata-transport.c | 24 +++++++++++++++++++++-- > include/linux/libata.h | 36 ++++++++++++++++++++++++++++++++++ > 3 files changed, 75 insertions(+), 2 deletions(-) > > diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig > index f6e943c74001..9ebaa3c288dd 100644 > --- a/drivers/ata/Kconfig > +++ b/drivers/ata/Kconfig > @@ -51,6 +51,23 @@ config ATA_VERBOSE_ERROR > > If unsure, say Y. > > +config ATA_SYSFS_NAMING > + bool "Align sysfs device names with kernel messages" > + default n > + help > + This option modifies the device naming to use a common > + format for all ATA objects (port, link, and devices) and use > + these names for both the sysfs directories and the kernel message > + log to align with the linux driver model. > + The names have the format ata<port>-<link>.<device>, such that an > + ATA port will have the name 'ata<port>', an ATA link the name > + 'ata<port>-<link>', and an ATA device the name > + 'ata<port>-<link>.<devno>'. > + This option changes the sysfs names, so userland tools might be > + impacted. > + > + If unsure, say N. > + > config ATA_FORCE > bool "\"libata.force=\" kernel parameter support" if EXPERT > default y > diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c > index ca129854a88c..b537eadc6501 100644 > --- a/drivers/ata/libata-transport.c > +++ b/drivers/ata/libata-transport.c > @@ -81,10 +81,14 @@ struct ata_internal { > tdev_to_port((dev)->parent) > > > -/* Device objects are always created whit link objects */ > +/* Device objects are always created with link objects */ > static int ata_tdev_add(struct ata_device *dev); > static void ata_tdev_delete(struct ata_device *dev); > > +struct bus_type ata_bus_type = { > + .name = "ata", > +}; > + > > /* > * Hack to allow attributes of the same name in different objects. > @@ -288,7 +292,12 @@ int ata_tport_add(struct device *parent, > dev->parent = parent; > ata_host_get(ap->host); > dev->release = ata_tport_release; > +#ifdef CONFIG_ATA_SYSFS_NAMING > + dev->bus = &ata_bus_type; > + dev_set_name(dev, "port%d", ap->print_id); > +#else > dev_set_name(dev, "ata%d", ap->print_id); > +#endif > transport_setup_device(dev); > ata_acpi_bind_port(ap); > error = device_add(dev); > @@ -444,6 +453,9 @@ int ata_tlink_add(struct ata_link *link) > device_initialize(dev); > dev->parent = &ap->tdev; > dev->release = ata_tlink_release; > +#ifdef CONFIG_ATA_SYSFS_NAMING > + dev->bus = &ata_bus_type; > +#endif > if (ata_is_host_link(link)) > dev_set_name(dev, "link%d", ap->print_id); > else > @@ -695,8 +707,11 @@ static int ata_tdev_add(struct ata_device *ata_dev) > device_initialize(dev); > dev->parent = &link->tdev; > dev->release = ata_tdev_release; > +#ifdef CONFIG_ATA_SYSFS_NAMING > + dev->bus = &ata_bus_type; > +#endif > if (ata_is_host_link(link)) > - dev_set_name(dev, "dev%d.%d", ap->print_id,ata_dev->devno); > + 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); > > @@ -822,8 +837,13 @@ __init int libata_transport_init(void) > error = transport_class_register(&ata_dev_class); > if (error) > goto out_unregister_port; > + error = bus_register(&ata_bus_type); > + if (error) > + goto out_unregister_bus; > return 0; > > + out_unregister_bus: > + bus_unregister(&ata_bus_type); > out_unregister_port: > transport_class_unregister(&ata_port_class); > out_unregister_link: > diff --git a/include/linux/libata.h b/include/linux/libata.h > index c258f69106f4..ab2d404cde08 100644 > --- a/include/linux/libata.h > +++ b/include/linux/libata.h > @@ -1440,6 +1440,41 @@ static inline int sata_srst_pmp(struct ata_link *link) > return link->pmp; > } > > +#ifdef CONFIG_ATA_SYSFS_NAMING > +#define ata_port_err(ap, fmt, ...) \ > + dev_err(&ap->tdev, fmt, ##__VA_ARGS__) > +#define ata_port_warn(ap, fmt, ...) \ > + dev_warn(&ap->tdev, fmt, ##__VA_ARGS__) > +#define ata_port_notice(ap, fmt, ...) \ > + dev_notice(&ap->tdev, fmt, ##__VA_ARGS__) > +#define ata_port_info(ap, fmt, ...) \ > + dev_info(&ap->tdev, fmt, ##__VA_ARGS__) > +#define ata_port_dbg(ap, fmt, ...) \ > + dev_dbg(&ap->tdev, fmt, ##__VA_ARGS__) > + > +#define ata_link_err(link, fmt, ...) \ > + dev_err(&link->tdev, fmt, ##__VA_ARGS__) > +#define ata_link_warn(link, fmt, ...) \ > + dev_warn(&link->tdev, fmt, ##__VA_ARGS__) > +#define ata_link_notice(link, fmt, ...) \ > + dev_notice(&link->tdev, fmt, ##__VA_ARGS__) > +#define ata_link_info(link, fmt, ...) \ > + dev_info(&link->tdev, fmt, ##__VA_ARGS__) > +#define ata_link_dbg(link, fmt, ...) \ > + dev_dbg(&link->tdev, fmt, ##__VA_ARGS__) > + > +#define ata_dev_err(dev, fmt, ...) \ > + dev_err(&dev->tdev, fmt, ##__VA_ARGS__) > +#define ata_dev_warn(dev, fmt, ...) \ > + dev_warn(&dev->tdev, fmt, ##__VA_ARGS__) > +#define ata_dev_notice(dev, fmt, ...) \ > + dev_notice(&dev->tdev, fmt, ##__VA_ARGS__) > +#define ata_dev_info(dev, fmt, ...) \ > + dev_info(&dev->tdev, fmt, ##__VA_ARGS__) > +#define ata_dev_dbg(dev, fmt, ...) \ > + dev_dbg(&dev->tdev, fmt, ##__VA_ARGS__) > + > +#else > #define ata_port_printk(level, ap, fmt, ...) \ > pr_ ## level ("ata%u: " fmt, (ap)->print_id, ##__VA_ARGS__) > > @@ -1495,6 +1530,7 @@ do { \ > ata_dev_printk(info, dev, fmt, ##__VA_ARGS__) > #define ata_dev_dbg(dev, fmt, ...) \ > ata_dev_printk(debug, dev, fmt, ##__VA_ARGS__) > +#endif > > void ata_print_version(const struct device *dev, const char *version); > Before the patch, in sysfs: # tree /sys/class/ata_device/ /sys/class/ata_device/ ... |-- dev8.0 -> ../../devices/pci0000:00/0000:00:17.0/ata8/link8/dev8.0/ata_device/dev8.0 ... # tree /sys/class/ata_port/ /sys/class/ata_port/ ... |-- ata8 -> ../../devices/pci0000:00/0000:00:17.0/ata8/ata_port/ata8 ... # tree /sys/class/ata_link/ /sys/class/ata_link/ ... |-- link8 -> ../../devices/pci0000:00/0000:00:17.0/ata8/link8/ata_link/link8 ... And the dmesg: [ 20.713229] ata8: SATA max UDMA/133 abar m524288@0x9d200000 port 0x9d200380 irq 309 [ 21.024579] ata8: SATA link up 6.0 Gbps (SStatus 133 SControl 300) [ 21.025763] ata8.00: ATA-9: WDC WUH721414ALE604, LDGNW240, max UDMA/133 [ 21.035186] ata8.00: 27344764928 sectors, multi 16: LBA48 NCQ (depth 32), AA [ 21.377184] ata8.00: Features: NCQ-sndrcv NCQ-prio [ 21.432131] ata8.00: configured for UDMA/133 After the patch, in sysfs: # tree ata_device/ ata_device/ ... |-- dev8.0 -> ../../devices/pci0000:00/0000:00:17.0/port8/link8/dev8.0/ata_device/dev8.0 ... # tree ata_port/ ata_port/ ... |-- port8 -> ../../devices/pci0000:00/0000:00:17.0/port8/ata_port/port8 ... # tree ata_link/ ata_link/ ... |-- link8 -> ../../devices/pci0000:00/0000:00:17.0/port8/link8/ata_link/link8 ... And the dmesg: [ 20.973002] ata port8: SATA max UDMA/133 abar m524288@0x9d200000 port 0x9d200380 irq 308 [ 21.355104] ata link8: SATA link up 6.0 Gbps (SStatus 133 SControl 300) [ 21.412804] ata dev8.0: ATA-9: WDC WUH721414ALE604, LDGNW240, max UDMA/133 [ 22.646935] ata dev8.0: 27344764928 sectors, multi 16: LBA48 NCQ (depth 32), AA [ 22.680872] ata dev8.0: Features: NCQ-sndrcv NCQ-prio [ 22.748586] ata dev8.0: configured for UDMA/133 So for the dmesg changes, this looks all good to me. As readable as before, no real change. If anything, we now clearly see if the message if from ata_dev_xxx(), ata_port_xxx() or ata_link_XXX() functions. For sysfs changes, the real change is "ataX" becoming "portX" and the addition of the new ata bus: # tree /sys/bus/ata ... | |-- dev8.0 -> ../../../devices/pci0000:00/0000:00:17.0/port8/link8/dev8.0 ... | |-- link8 -> ../../../devices/pci0000:00/0000:00:17.0/port8/link8 ... | |-- port8 -> ../../../devices/pci0000:00/0000:00:17.0/port8 ... All good. And I wonder if we could not add a symbolic link named "ataX" pointing to "portX" (the new name) to avoid breaking userspace. That would allow dropping the Kconfig option. Hmm ?
On 1/3/22 18:09, Hannes Reinecke wrote: > This patch adds a config option ATA_SYSFS_NAMING to align the libata > device names in sysfs with those in the kernel message log. > It adds a new dummy bus 'ata', which collects all ata device objects > like ata_port, ata_link, and ata_dev, and adds an 'ata' prefix to the > message log. > For consistency the 'ata_dev' name has been changed from 'ata' to 'dev'; > as this induces a sysfs change the config option is disabled per default. > > Patch is relative to the 'for-5.17-logging' branch from Damien. FYI, I queued the logging series in for-5.17, minus this patch. Everything is in for-next too to check that nothing is broken. For this patch, as I commented, I think we can keep a backward compatible naming using sysfs symlinks. But I am not entirely sure if that can work with port-multiplier setups. Let's work on that for the next cycle ? > > Signed-off-by: Hannes Reinecke <hare@suse.de> > --- > drivers/ata/Kconfig | 17 ++++++++++++++++ > drivers/ata/libata-transport.c | 24 +++++++++++++++++++++-- > include/linux/libata.h | 36 ++++++++++++++++++++++++++++++++++ > 3 files changed, 75 insertions(+), 2 deletions(-) > > diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig > index f6e943c74001..9ebaa3c288dd 100644 > --- a/drivers/ata/Kconfig > +++ b/drivers/ata/Kconfig > @@ -51,6 +51,23 @@ config ATA_VERBOSE_ERROR > > If unsure, say Y. > > +config ATA_SYSFS_NAMING > + bool "Align sysfs device names with kernel messages" > + default n > + help > + This option modifies the device naming to use a common > + format for all ATA objects (port, link, and devices) and use > + these names for both the sysfs directories and the kernel message > + log to align with the linux driver model. > + The names have the format ata<port>-<link>.<device>, such that an > + ATA port will have the name 'ata<port>', an ATA link the name > + 'ata<port>-<link>', and an ATA device the name > + 'ata<port>-<link>.<devno>'. > + This option changes the sysfs names, so userland tools might be > + impacted. > + > + If unsure, say N. > + > config ATA_FORCE > bool "\"libata.force=\" kernel parameter support" if EXPERT > default y > diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c > index ca129854a88c..b537eadc6501 100644 > --- a/drivers/ata/libata-transport.c > +++ b/drivers/ata/libata-transport.c > @@ -81,10 +81,14 @@ struct ata_internal { > tdev_to_port((dev)->parent) > > > -/* Device objects are always created whit link objects */ > +/* Device objects are always created with link objects */ > static int ata_tdev_add(struct ata_device *dev); > static void ata_tdev_delete(struct ata_device *dev); > > +struct bus_type ata_bus_type = { > + .name = "ata", > +}; > + > > /* > * Hack to allow attributes of the same name in different objects. > @@ -288,7 +292,12 @@ int ata_tport_add(struct device *parent, > dev->parent = parent; > ata_host_get(ap->host); > dev->release = ata_tport_release; > +#ifdef CONFIG_ATA_SYSFS_NAMING > + dev->bus = &ata_bus_type; > + dev_set_name(dev, "port%d", ap->print_id); > +#else > dev_set_name(dev, "ata%d", ap->print_id); > +#endif > transport_setup_device(dev); > ata_acpi_bind_port(ap); > error = device_add(dev); > @@ -444,6 +453,9 @@ int ata_tlink_add(struct ata_link *link) > device_initialize(dev); > dev->parent = &ap->tdev; > dev->release = ata_tlink_release; > +#ifdef CONFIG_ATA_SYSFS_NAMING > + dev->bus = &ata_bus_type; > +#endif > if (ata_is_host_link(link)) > dev_set_name(dev, "link%d", ap->print_id); > else > @@ -695,8 +707,11 @@ static int ata_tdev_add(struct ata_device *ata_dev) > device_initialize(dev); > dev->parent = &link->tdev; > dev->release = ata_tdev_release; > +#ifdef CONFIG_ATA_SYSFS_NAMING > + dev->bus = &ata_bus_type; > +#endif > if (ata_is_host_link(link)) > - dev_set_name(dev, "dev%d.%d", ap->print_id,ata_dev->devno); > + 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); > > @@ -822,8 +837,13 @@ __init int libata_transport_init(void) > error = transport_class_register(&ata_dev_class); > if (error) > goto out_unregister_port; > + error = bus_register(&ata_bus_type); > + if (error) > + goto out_unregister_bus; > return 0; > > + out_unregister_bus: > + bus_unregister(&ata_bus_type); > out_unregister_port: > transport_class_unregister(&ata_port_class); > out_unregister_link: > diff --git a/include/linux/libata.h b/include/linux/libata.h > index c258f69106f4..ab2d404cde08 100644 > --- a/include/linux/libata.h > +++ b/include/linux/libata.h > @@ -1440,6 +1440,41 @@ static inline int sata_srst_pmp(struct ata_link *link) > return link->pmp; > } > > +#ifdef CONFIG_ATA_SYSFS_NAMING > +#define ata_port_err(ap, fmt, ...) \ > + dev_err(&ap->tdev, fmt, ##__VA_ARGS__) > +#define ata_port_warn(ap, fmt, ...) \ > + dev_warn(&ap->tdev, fmt, ##__VA_ARGS__) > +#define ata_port_notice(ap, fmt, ...) \ > + dev_notice(&ap->tdev, fmt, ##__VA_ARGS__) > +#define ata_port_info(ap, fmt, ...) \ > + dev_info(&ap->tdev, fmt, ##__VA_ARGS__) > +#define ata_port_dbg(ap, fmt, ...) \ > + dev_dbg(&ap->tdev, fmt, ##__VA_ARGS__) > + > +#define ata_link_err(link, fmt, ...) \ > + dev_err(&link->tdev, fmt, ##__VA_ARGS__) > +#define ata_link_warn(link, fmt, ...) \ > + dev_warn(&link->tdev, fmt, ##__VA_ARGS__) > +#define ata_link_notice(link, fmt, ...) \ > + dev_notice(&link->tdev, fmt, ##__VA_ARGS__) > +#define ata_link_info(link, fmt, ...) \ > + dev_info(&link->tdev, fmt, ##__VA_ARGS__) > +#define ata_link_dbg(link, fmt, ...) \ > + dev_dbg(&link->tdev, fmt, ##__VA_ARGS__) > + > +#define ata_dev_err(dev, fmt, ...) \ > + dev_err(&dev->tdev, fmt, ##__VA_ARGS__) > +#define ata_dev_warn(dev, fmt, ...) \ > + dev_warn(&dev->tdev, fmt, ##__VA_ARGS__) > +#define ata_dev_notice(dev, fmt, ...) \ > + dev_notice(&dev->tdev, fmt, ##__VA_ARGS__) > +#define ata_dev_info(dev, fmt, ...) \ > + dev_info(&dev->tdev, fmt, ##__VA_ARGS__) > +#define ata_dev_dbg(dev, fmt, ...) \ > + dev_dbg(&dev->tdev, fmt, ##__VA_ARGS__) > + > +#else > #define ata_port_printk(level, ap, fmt, ...) \ > pr_ ## level ("ata%u: " fmt, (ap)->print_id, ##__VA_ARGS__) > > @@ -1495,6 +1530,7 @@ do { \ > ata_dev_printk(info, dev, fmt, ##__VA_ARGS__) > #define ata_dev_dbg(dev, fmt, ...) \ > ata_dev_printk(debug, dev, fmt, ##__VA_ARGS__) > +#endif > > void ata_print_version(const struct device *dev, const char *version); >
On 1/5/22 07:18, Damien Le Moal wrote: > On 1/3/22 18:09, Hannes Reinecke wrote: >> This patch adds a config option ATA_SYSFS_NAMING to align the libata >> device names in sysfs with those in the kernel message log. >> It adds a new dummy bus 'ata', which collects all ata device objects >> like ata_port, ata_link, and ata_dev, and adds an 'ata' prefix to the >> message log. >> For consistency the 'ata_dev' name has been changed from 'ata' to 'dev'; >> as this induces a sysfs change the config option is disabled per default. >> >> Patch is relative to the 'for-5.17-logging' branch from Damien. > > FYI, I queued the logging series in for-5.17, minus this patch. > Everything is in for-next too to check that nothing is broken. > > For this patch, as I commented, I think we can keep a backward > compatible naming using sysfs symlinks. But I am not entirely sure if > that can work with port-multiplier setups. Let's work on that for the > next cycle ? > Well, I'm not terribly happy about the current port multiplier implementation, either. Currently they are named 'ataX.Y.0', making them the only ata objects with three levels. Personally I would take the PATA master/slave thingie as an example, and just increase the last number (such that we would have ata1.0, ata1.1, ata1.2 etc). Reasoning here is that PMP is pretty much an SATA thing, and 'slave' drives is a PATA thing. So they'll never clash. As for the 'ataX.Y' link; that'll require even more sysfs trickery. Let's see if I can come up with something. So yeah, let's hold off the patch for now. First I need to get a PMP cable to test things out :-) Cheers, Hannes
On 1/5/22 16:41, Hannes Reinecke wrote: > On 1/5/22 07:18, Damien Le Moal wrote: >> On 1/3/22 18:09, Hannes Reinecke wrote: >>> This patch adds a config option ATA_SYSFS_NAMING to align the libata >>> device names in sysfs with those in the kernel message log. >>> It adds a new dummy bus 'ata', which collects all ata device objects >>> like ata_port, ata_link, and ata_dev, and adds an 'ata' prefix to the >>> message log. >>> For consistency the 'ata_dev' name has been changed from 'ata' to 'dev'; >>> as this induces a sysfs change the config option is disabled per default. >>> >>> Patch is relative to the 'for-5.17-logging' branch from Damien. >> >> FYI, I queued the logging series in for-5.17, minus this patch. >> Everything is in for-next too to check that nothing is broken. >> >> For this patch, as I commented, I think we can keep a backward >> compatible naming using sysfs symlinks. But I am not entirely sure if >> that can work with port-multiplier setups. Let's work on that for the >> next cycle ? >> > Well, I'm not terribly happy about the current port multiplier > implementation, either. > Currently they are named 'ataX.Y.0', making them the only ata objects > with three levels. Personally I would take the PATA master/slave thingie > as an example, and just increase the last number (such that we would > have ata1.0, ata1.1, ata1.2 etc). > Reasoning here is that PMP is pretty much an SATA thing, and 'slave' > drives is a PATA thing. So they'll never clash. Sounds sane to me. > > As for the 'ataX.Y' link; that'll require even more sysfs trickery. Hmm, as long as we can create sysfs paths that are compatible with the old naming scheme (which seem to differ only for the root port object name), it may be as simple as calling sysfs_create_link() for the port objects ? > Let's see if I can come up with something. > So yeah, let's hold off the patch for now. > > First I need to get a PMP cable to test things out :-) I do have some in the lab, so I can test. Just need to go to the lab, for once :) > > Cheers, > > Hannes
diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig index f6e943c74001..9ebaa3c288dd 100644 --- a/drivers/ata/Kconfig +++ b/drivers/ata/Kconfig @@ -51,6 +51,23 @@ config ATA_VERBOSE_ERROR If unsure, say Y. +config ATA_SYSFS_NAMING + bool "Align sysfs device names with kernel messages" + default n + help + This option modifies the device naming to use a common + format for all ATA objects (port, link, and devices) and use + these names for both the sysfs directories and the kernel message + log to align with the linux driver model. + The names have the format ata<port>-<link>.<device>, such that an + ATA port will have the name 'ata<port>', an ATA link the name + 'ata<port>-<link>', and an ATA device the name + 'ata<port>-<link>.<devno>'. + This option changes the sysfs names, so userland tools might be + impacted. + + If unsure, say N. + config ATA_FORCE bool "\"libata.force=\" kernel parameter support" if EXPERT default y diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c index ca129854a88c..b537eadc6501 100644 --- a/drivers/ata/libata-transport.c +++ b/drivers/ata/libata-transport.c @@ -81,10 +81,14 @@ struct ata_internal { tdev_to_port((dev)->parent) -/* Device objects are always created whit link objects */ +/* Device objects are always created with link objects */ static int ata_tdev_add(struct ata_device *dev); static void ata_tdev_delete(struct ata_device *dev); +struct bus_type ata_bus_type = { + .name = "ata", +}; + /* * Hack to allow attributes of the same name in different objects. @@ -288,7 +292,12 @@ int ata_tport_add(struct device *parent, dev->parent = parent; ata_host_get(ap->host); dev->release = ata_tport_release; +#ifdef CONFIG_ATA_SYSFS_NAMING + dev->bus = &ata_bus_type; + dev_set_name(dev, "port%d", ap->print_id); +#else dev_set_name(dev, "ata%d", ap->print_id); +#endif transport_setup_device(dev); ata_acpi_bind_port(ap); error = device_add(dev); @@ -444,6 +453,9 @@ int ata_tlink_add(struct ata_link *link) device_initialize(dev); dev->parent = &ap->tdev; dev->release = ata_tlink_release; +#ifdef CONFIG_ATA_SYSFS_NAMING + dev->bus = &ata_bus_type; +#endif if (ata_is_host_link(link)) dev_set_name(dev, "link%d", ap->print_id); else @@ -695,8 +707,11 @@ static int ata_tdev_add(struct ata_device *ata_dev) device_initialize(dev); dev->parent = &link->tdev; dev->release = ata_tdev_release; +#ifdef CONFIG_ATA_SYSFS_NAMING + dev->bus = &ata_bus_type; +#endif if (ata_is_host_link(link)) - dev_set_name(dev, "dev%d.%d", ap->print_id,ata_dev->devno); + 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); @@ -822,8 +837,13 @@ __init int libata_transport_init(void) error = transport_class_register(&ata_dev_class); if (error) goto out_unregister_port; + error = bus_register(&ata_bus_type); + if (error) + goto out_unregister_bus; return 0; + out_unregister_bus: + bus_unregister(&ata_bus_type); out_unregister_port: transport_class_unregister(&ata_port_class); out_unregister_link: diff --git a/include/linux/libata.h b/include/linux/libata.h index c258f69106f4..ab2d404cde08 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -1440,6 +1440,41 @@ static inline int sata_srst_pmp(struct ata_link *link) return link->pmp; } +#ifdef CONFIG_ATA_SYSFS_NAMING +#define ata_port_err(ap, fmt, ...) \ + dev_err(&ap->tdev, fmt, ##__VA_ARGS__) +#define ata_port_warn(ap, fmt, ...) \ + dev_warn(&ap->tdev, fmt, ##__VA_ARGS__) +#define ata_port_notice(ap, fmt, ...) \ + dev_notice(&ap->tdev, fmt, ##__VA_ARGS__) +#define ata_port_info(ap, fmt, ...) \ + dev_info(&ap->tdev, fmt, ##__VA_ARGS__) +#define ata_port_dbg(ap, fmt, ...) \ + dev_dbg(&ap->tdev, fmt, ##__VA_ARGS__) + +#define ata_link_err(link, fmt, ...) \ + dev_err(&link->tdev, fmt, ##__VA_ARGS__) +#define ata_link_warn(link, fmt, ...) \ + dev_warn(&link->tdev, fmt, ##__VA_ARGS__) +#define ata_link_notice(link, fmt, ...) \ + dev_notice(&link->tdev, fmt, ##__VA_ARGS__) +#define ata_link_info(link, fmt, ...) \ + dev_info(&link->tdev, fmt, ##__VA_ARGS__) +#define ata_link_dbg(link, fmt, ...) \ + dev_dbg(&link->tdev, fmt, ##__VA_ARGS__) + +#define ata_dev_err(dev, fmt, ...) \ + dev_err(&dev->tdev, fmt, ##__VA_ARGS__) +#define ata_dev_warn(dev, fmt, ...) \ + dev_warn(&dev->tdev, fmt, ##__VA_ARGS__) +#define ata_dev_notice(dev, fmt, ...) \ + dev_notice(&dev->tdev, fmt, ##__VA_ARGS__) +#define ata_dev_info(dev, fmt, ...) \ + dev_info(&dev->tdev, fmt, ##__VA_ARGS__) +#define ata_dev_dbg(dev, fmt, ...) \ + dev_dbg(&dev->tdev, fmt, ##__VA_ARGS__) + +#else #define ata_port_printk(level, ap, fmt, ...) \ pr_ ## level ("ata%u: " fmt, (ap)->print_id, ##__VA_ARGS__) @@ -1495,6 +1530,7 @@ do { \ ata_dev_printk(info, dev, fmt, ##__VA_ARGS__) #define ata_dev_dbg(dev, fmt, ...) \ ata_dev_printk(debug, dev, fmt, ##__VA_ARGS__) +#endif void ata_print_version(const struct device *dev, const char *version);
This patch adds a config option ATA_SYSFS_NAMING to align the libata device names in sysfs with those in the kernel message log. It adds a new dummy bus 'ata', which collects all ata device objects like ata_port, ata_link, and ata_dev, and adds an 'ata' prefix to the message log. For consistency the 'ata_dev' name has been changed from 'ata' to 'dev'; as this induces a sysfs change the config option is disabled per default. Patch is relative to the 'for-5.17-logging' branch from Damien. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/ata/Kconfig | 17 ++++++++++++++++ drivers/ata/libata-transport.c | 24 +++++++++++++++++++++-- include/linux/libata.h | 36 ++++++++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 2 deletions(-)