* [PATCHv4 0/2] libata: sysfs naming @ 2022-03-25 12:56 Hannes Reinecke 2022-03-25 12:56 ` [PATCH 1/2] libata: rework " Hannes Reinecke 2022-03-25 12:56 ` [PATCH 2/2] libata: CONFIG_ATA_SYSFS_COMPAT Hannes Reinecke 0 siblings, 2 replies; 7+ messages in thread From: Hannes Reinecke @ 2022-03-25 12:56 UTC (permalink / raw) To: Damien LeMoal; +Cc: linux-ide, Hannes Reinecke Hi all, here's my second attempt the align the libata object naming with sysfs. Key point is to introduce an 'ata' bus, which serves to collect all libata object (ata_port, ata_link, and ata_device). To facilitate that the name of the 'ata_port' object changes from 'ata' to 'port'. To provide backwards compatibility I've added config option CONFIG_ATA_SYSFS_COMPAT to create a compat symlink in the PCI sysfs device directory with the name of 'ata'. As usual, comments and reviews are welcome. Changes to v3: - Included reviews from Damien Changes to v2: - Dropped patch to change PMP naming - created compability symlink instead of full sysfs objects Hannes Reinecke (2): libata: rework sysfs naming libata: CONFIG_ATA_SYSFS_COMPAT drivers/ata/Kconfig | 10 ++++++ drivers/ata/libata-transport.c | 62 ++++++++++++++++++++++++++++++++-- include/linux/libata.h | 53 ++++++++--------------------- 3 files changed, 84 insertions(+), 41 deletions(-) -- 2.29.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] libata: rework sysfs naming 2022-03-25 12:56 [PATCHv4 0/2] libata: sysfs naming Hannes Reinecke @ 2022-03-25 12:56 ` Hannes Reinecke 2022-03-26 18:08 ` [RFC PATCH] libata: ata_bus_type can be static kernel test robot 2022-03-26 18:12 ` [PATCH 1/2] libata: rework sysfs naming kernel test robot 2022-03-25 12:56 ` [PATCH 2/2] libata: CONFIG_ATA_SYSFS_COMPAT Hannes Reinecke 1 sibling, 2 replies; 7+ messages in thread From: Hannes Reinecke @ 2022-03-25 12:56 UTC (permalink / raw) To: Damien LeMoal; +Cc: linux-ide, Hannes Reinecke This patch 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. To be consistent with the other libata objects the 'ata_port' object name has been changed from 'ata' to 'port'. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/ata/libata-transport.c | 21 ++++++++++++-- include/linux/libata.h | 53 ++++++++++------------------------ 2 files changed, 33 insertions(+), 41 deletions(-) diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c index ca129854a88c..e5ed5046b299 100644 --- a/drivers/ata/libata-transport.c +++ b/drivers/ata/libata-transport.c @@ -81,10 +81,13 @@ 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 +291,9 @@ int ata_tport_add(struct device *parent, dev->parent = parent; ata_host_get(ap->host); dev->release = ata_tport_release; - dev_set_name(dev, "ata%d", ap->print_id); + dev->bus = &ata_bus_type; + dev_set_name(dev, "port%d", ap->print_id); + transport_setup_device(dev); ata_acpi_bind_port(ap); error = device_add(dev); @@ -444,6 +449,8 @@ int ata_tlink_add(struct ata_link *link) device_initialize(dev); dev->parent = &ap->tdev; dev->release = ata_tlink_release; + dev->bus = &ata_bus_type; + if (ata_is_host_link(link)) dev_set_name(dev, "link%d", ap->print_id); else @@ -695,8 +702,10 @@ static int ata_tdev_add(struct ata_device *ata_dev) device_initialize(dev); dev->parent = &link->tdev; dev->release = ata_tdev_release; + dev->bus = &ata_bus_type; + 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 +831,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_dev; return 0; + out_unregister_dev: + transport_class_unregister(&ata_dev_class); out_unregister_port: transport_class_unregister(&ata_port_class); out_unregister_link: @@ -835,6 +849,7 @@ __init int libata_transport_init(void) void __exit libata_transport_exit(void) { + bus_unregister(&ata_bus_type); transport_class_unregister(&ata_link_class); transport_class_unregister(&ata_port_class); transport_class_unregister(&ata_dev_class); diff --git a/include/linux/libata.h b/include/linux/libata.h index 0619ae462ecd..7dc06e5cbc3a 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -1458,61 +1458,38 @@ static inline int sata_srst_pmp(struct ata_link *link) return link->pmp; } -#define ata_port_printk(level, ap, fmt, ...) \ - pr_ ## level ("ata%u: " fmt, (ap)->print_id, ##__VA_ARGS__) - #define ata_port_err(ap, fmt, ...) \ - ata_port_printk(err, ap, fmt, ##__VA_ARGS__) + dev_err(&ap->tdev, fmt, ##__VA_ARGS__) #define ata_port_warn(ap, fmt, ...) \ - ata_port_printk(warn, ap, fmt, ##__VA_ARGS__) + dev_warn(&ap->tdev, fmt, ##__VA_ARGS__) #define ata_port_notice(ap, fmt, ...) \ - ata_port_printk(notice, ap, fmt, ##__VA_ARGS__) + dev_notice(&ap->tdev, fmt, ##__VA_ARGS__) #define ata_port_info(ap, fmt, ...) \ - ata_port_printk(info, ap, fmt, ##__VA_ARGS__) + dev_info(&ap->tdev, fmt, ##__VA_ARGS__) #define ata_port_dbg(ap, fmt, ...) \ - ata_port_printk(debug, ap, fmt, ##__VA_ARGS__) - -#define ata_link_printk(level, link, fmt, ...) \ -do { \ - if (sata_pmp_attached((link)->ap) || \ - (link)->ap->slave_link) \ - pr_ ## level ("ata%u.%02u: " fmt, \ - (link)->ap->print_id, \ - (link)->pmp, \ - ##__VA_ARGS__); \ - else \ - pr_ ## level ("ata%u: " fmt, \ - (link)->ap->print_id, \ - ##__VA_ARGS__); \ -} while (0) + dev_dbg(&ap->tdev, fmt, ##__VA_ARGS__) #define ata_link_err(link, fmt, ...) \ - ata_link_printk(err, link, fmt, ##__VA_ARGS__) + dev_err(&link->tdev, fmt, ##__VA_ARGS__) #define ata_link_warn(link, fmt, ...) \ - ata_link_printk(warn, link, fmt, ##__VA_ARGS__) + dev_warn(&link->tdev, fmt, ##__VA_ARGS__) #define ata_link_notice(link, fmt, ...) \ - ata_link_printk(notice, link, fmt, ##__VA_ARGS__) + dev_notice(&link->tdev, fmt, ##__VA_ARGS__) #define ata_link_info(link, fmt, ...) \ - ata_link_printk(info, link, fmt, ##__VA_ARGS__) + dev_info(&link->tdev, fmt, ##__VA_ARGS__) #define ata_link_dbg(link, fmt, ...) \ - ata_link_printk(debug, link, fmt, ##__VA_ARGS__) - -#define ata_dev_printk(level, dev, fmt, ...) \ - pr_ ## level("ata%u.%02u: " fmt, \ - (dev)->link->ap->print_id, \ - (dev)->link->pmp + (dev)->devno, \ - ##__VA_ARGS__) + dev_dbg(&link->tdev, fmt, ##__VA_ARGS__) #define ata_dev_err(dev, fmt, ...) \ - ata_dev_printk(err, dev, fmt, ##__VA_ARGS__) + dev_err(&dev->tdev, fmt, ##__VA_ARGS__) #define ata_dev_warn(dev, fmt, ...) \ - ata_dev_printk(warn, dev, fmt, ##__VA_ARGS__) + dev_warn(&dev->tdev, fmt, ##__VA_ARGS__) #define ata_dev_notice(dev, fmt, ...) \ - ata_dev_printk(notice, dev, fmt, ##__VA_ARGS__) + dev_notice(&dev->tdev, fmt, ##__VA_ARGS__) #define ata_dev_info(dev, fmt, ...) \ - ata_dev_printk(info, dev, fmt, ##__VA_ARGS__) + dev_info(&dev->tdev, fmt, ##__VA_ARGS__) #define ata_dev_dbg(dev, fmt, ...) \ - ata_dev_printk(debug, dev, fmt, ##__VA_ARGS__) + dev_dbg(&dev->tdev, fmt, ##__VA_ARGS__) void ata_print_version(const struct device *dev, const char *version); -- 2.29.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [RFC PATCH] libata: ata_bus_type can be static 2022-03-25 12:56 ` [PATCH 1/2] libata: rework " Hannes Reinecke @ 2022-03-26 18:08 ` kernel test robot 2022-03-26 18:12 ` [PATCH 1/2] libata: rework sysfs naming kernel test robot 1 sibling, 0 replies; 7+ messages in thread From: kernel test robot @ 2022-03-26 18:08 UTC (permalink / raw) To: Hannes Reinecke, Damien LeMoal; +Cc: kbuild-all, linux-ide, Hannes Reinecke drivers/ata/libata-transport.c:88:17: warning: symbol 'ata_bus_type' was not declared. Should it be static? Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: kernel test robot <lkp@intel.com> --- drivers/ata/libata-transport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c index e5ed5046b299e2..b9412d1fe3e083 100644 --- a/drivers/ata/libata-transport.c +++ b/drivers/ata/libata-transport.c @@ -85,7 +85,7 @@ struct ata_internal { static int ata_tdev_add(struct ata_device *dev); static void ata_tdev_delete(struct ata_device *dev); -struct bus_type ata_bus_type = { +static struct bus_type ata_bus_type = { .name = "ata", }; ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] libata: rework sysfs naming 2022-03-25 12:56 ` [PATCH 1/2] libata: rework " Hannes Reinecke 2022-03-26 18:08 ` [RFC PATCH] libata: ata_bus_type can be static kernel test robot @ 2022-03-26 18:12 ` kernel test robot 1 sibling, 0 replies; 7+ messages in thread From: kernel test robot @ 2022-03-26 18:12 UTC (permalink / raw) To: Hannes Reinecke, Damien LeMoal; +Cc: kbuild-all, linux-ide, Hannes Reinecke Hi Hannes, I love your patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on v5.17 next-20220325] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Hannes-Reinecke/libata-sysfs-naming/20220325-205722 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 34af78c4e616c359ed428d79fe4758a35d2c5473 config: x86_64-rhel-8.3-kselftests (https://download.01.org/0day-ci/archive/20220327/202203270220.q1Q39xe4-lkp@intel.com/config) compiler: gcc-9 (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0 reproduce: # apt-get install sparse # sparse version: v0.6.4-dirty # https://github.com/0day-ci/linux/commit/ba4273174499b7351d34ffc22021d0c382cb1d63 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Hannes-Reinecke/libata-sysfs-naming/20220325-205722 git checkout ba4273174499b7351d34ffc22021d0c382cb1d63 # save the config file to linux build tree mkdir build_dir make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/ata/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> sparse warnings: (new ones prefixed by >>) >> drivers/ata/libata-transport.c:88:17: sparse: sparse: symbol 'ata_bus_type' was not declared. Should it be static? Please review and possibly fold the followup patch. -- 0-DAY CI Kernel Test Service https://01.org/lkp ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] libata: CONFIG_ATA_SYSFS_COMPAT 2022-03-25 12:56 [PATCHv4 0/2] libata: sysfs naming Hannes Reinecke 2022-03-25 12:56 ` [PATCH 1/2] libata: rework " Hannes Reinecke @ 2022-03-25 12:56 ` Hannes Reinecke 2022-03-25 16:22 ` Sergey Shtylyov 1 sibling, 1 reply; 7+ messages in thread From: Hannes Reinecke @ 2022-03-25 12:56 UTC (permalink / raw) To: Damien LeMoal; +Cc: linux-ide, Hannes Reinecke Add a config option 'ATA_SYSFS_COMPAT' to create a compatibility 'ata' symlink in the PCI device sysfs directory. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/ata/Kconfig | 10 +++++++++ drivers/ata/libata-transport.c | 41 ++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig index e5641e6c52ee..f27b12ba2ce7 100644 --- a/drivers/ata/Kconfig +++ b/drivers/ata/Kconfig @@ -51,6 +51,16 @@ config ATA_VERBOSE_ERROR If unsure, say Y. +config ATA_SYSFS_COMPAT + bool "Keep original sysfs layout" + default y + help + This option retains the original sysfs layout by adding an + additional ata_port object with the name of 'ataX' in + to the ATA objects like 'ata_port', 'ata_link', and 'ata_device'. + + If unsure, say Y. + 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 e5ed5046b299..29dec89ccc2d 100644 --- a/drivers/ata/libata-transport.c +++ b/drivers/ata/libata-transport.c @@ -251,6 +251,39 @@ static int ata_tport_match(struct attribute_container *cont, return &ata_scsi_transport_template->host_attrs.ac == cont; } +#ifdef CONFIG_ATA_SYSFS_COMPAT +static int ata_tport_compat_link_add(struct ata_port *ap) +{ + struct device *dev = &ap->tdev; + struct device *parent = dev->parent; + char compat_name[64]; + + sprintf(compat_name, "ata%d", ap->print_id); + + return sysfs_create_link(&parent->kobj, &dev->kobj, compat_name); +} + +static void ata_tport_compat_link_delete(struct ata_port *ap) +{ + struct device *dev = &ap->tdev; + struct device *parent = dev->parent; + char compat_name[64]; + + sprintf(compat_name, "ata%d", ap->print_id); + sysfs_remove_link(&parent->kobj, compat_name); +} + +#else + +static inline int ata_tport_compat_link_add(struct ata_port *ap) +{ + return 0; +} + +static inline void ata_tport_compat_link_delete(struct ata_port *ap) {} + +#endif + /** * ata_tport_delete -- remove ATA PORT * @ap: ATA PORT to remove @@ -261,6 +294,7 @@ void ata_tport_delete(struct ata_port *ap) { struct device *dev = &ap->tdev; + ata_tport_compat_link_delete(ap); ata_tlink_delete(&ap->link); transport_remove_device(dev); @@ -313,8 +347,15 @@ int ata_tport_add(struct device *parent, if (error) { goto tport_link_err; } + + error = ata_tport_compat_link_add(ap); + if (error) + goto compat_link_err; + return 0; + compat_link_err: + ata_tlink_delete(&ap->link); tport_link_err: transport_remove_device(dev); device_del(dev); -- 2.29.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] libata: CONFIG_ATA_SYSFS_COMPAT 2022-03-25 12:56 ` [PATCH 2/2] libata: CONFIG_ATA_SYSFS_COMPAT Hannes Reinecke @ 2022-03-25 16:22 ` Sergey Shtylyov 2022-03-25 16:29 ` Hannes Reinecke 0 siblings, 1 reply; 7+ messages in thread From: Sergey Shtylyov @ 2022-03-25 16:22 UTC (permalink / raw) To: Hannes Reinecke, Damien LeMoal; +Cc: linux-ide Hello! On 3/25/22 3:56 PM, Hannes Reinecke wrote: > Add a config option 'ATA_SYSFS_COMPAT' to create a compatibility > 'ata' symlink in the PCI device sysfs directory. > > Signed-off-by: Hannes Reinecke <hare@suse.de> [...] > diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c > index e5ed5046b299..29dec89ccc2d 100644 > --- a/drivers/ata/libata-transport.c > +++ b/drivers/ata/libata-transport.c > @@ -251,6 +251,39 @@ static int ata_tport_match(struct attribute_container *cont, > return &ata_scsi_transport_template->host_attrs.ac == cont; > } > > +#ifdef CONFIG_ATA_SYSFS_COMPAT > +static int ata_tport_compat_link_add(struct ata_port *ap) > +{ > + struct device *dev = &ap->tdev; Indent with a tab, please. > + struct device *parent = dev->parent; > + char compat_name[64]; Same here. The buffer seems oversized too... > + > + sprintf(compat_name, "ata%d", ap->print_id); snprintf(), perhaps? > + > + return sysfs_create_link(&parent->kobj, &dev->kobj, compat_name); > +} > + > +static void ata_tport_compat_link_delete(struct ata_port *ap) > +{ > + struct device *dev = &ap->tdev; > + struct device *parent = dev->parent; > + char compat_name[64]; > + > + sprintf(compat_name, "ata%d", ap->print_id); snprintf()? [...] MBR, Sergey ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] libata: CONFIG_ATA_SYSFS_COMPAT 2022-03-25 16:22 ` Sergey Shtylyov @ 2022-03-25 16:29 ` Hannes Reinecke 0 siblings, 0 replies; 7+ messages in thread From: Hannes Reinecke @ 2022-03-25 16:29 UTC (permalink / raw) To: Sergey Shtylyov, Damien LeMoal; +Cc: linux-ide On 3/25/22 17:22, Sergey Shtylyov wrote: > Hello! > > On 3/25/22 3:56 PM, Hannes Reinecke wrote: > >> Add a config option 'ATA_SYSFS_COMPAT' to create a compatibility >> 'ata' symlink in the PCI device sysfs directory. >> >> Signed-off-by: Hannes Reinecke <hare@suse.de> > [...] >> diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c >> index e5ed5046b299..29dec89ccc2d 100644 >> --- a/drivers/ata/libata-transport.c >> +++ b/drivers/ata/libata-transport.c >> @@ -251,6 +251,39 @@ static int ata_tport_match(struct attribute_container *cont, >> return &ata_scsi_transport_template->host_attrs.ac == cont; >> } >> >> +#ifdef CONFIG_ATA_SYSFS_COMPAT >> +static int ata_tport_compat_link_add(struct ata_port *ap) >> +{ >> + struct device *dev = &ap->tdev; > > Indent with a tab, please. > Of course. >> + struct device *parent = dev->parent; >> + char compat_name[64]; > > Same here. The buffer seems oversized too... > Hard to tell how many ATA devices there are in the system. More than hundred? But yeah, I can reduce it. >> + >> + sprintf(compat_name, "ata%d", ap->print_id); > > snprintf(), perhaps? > OK. Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-03-26 18:13 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-03-25 12:56 [PATCHv4 0/2] libata: sysfs naming Hannes Reinecke 2022-03-25 12:56 ` [PATCH 1/2] libata: rework " Hannes Reinecke 2022-03-26 18:08 ` [RFC PATCH] libata: ata_bus_type can be static kernel test robot 2022-03-26 18:12 ` [PATCH 1/2] libata: rework sysfs naming kernel test robot 2022-03-25 12:56 ` [PATCH 2/2] libata: CONFIG_ATA_SYSFS_COMPAT Hannes Reinecke 2022-03-25 16:22 ` Sergey Shtylyov 2022-03-25 16:29 ` Hannes Reinecke
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox