| Message ID | 20220321145659.97150-3-hare@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | libata: sysfs naming | expand |
On 3/21/22 23:56, Hannes Reinecke wrote: > Add a config option 'ATA_SYSFS_COMPAT' to allow the user to select > whether the compability 'ata_port' object with the name of 'ata' > should be registered with sysfs. > > Signed-off-by: Hannes Reinecke <hare@suse.de> > --- > drivers/ata/Kconfig | 10 ++++++++++ > drivers/ata/libata-transport.c | 19 ++++++++++++++++--- > 2 files changed, 26 insertions(+), 3 deletions(-) > > diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig > index cb54631fd950..fe42a246d21d 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 5169a5db141d..efca41039d1e 100644 > --- a/drivers/ata/libata-transport.c > +++ b/drivers/ata/libata-transport.c > @@ -261,15 +261,21 @@ static int ata_tport_match(struct attribute_container *cont, > void ata_tport_delete(struct ata_port *ap) > { > struct device *dev = &ap->tdev; > +#ifdef CONFIG_ATA_SYSFS_COMPAT > struct device *cdev = &ap->cdev; > +#endif > > ata_tlink_delete(&ap->link); > > +#ifdef CONFIG_ATA_SYSFS_COMPAT > device_del(cdev); > +#endif > transport_remove_device(dev); > device_del(dev); > transport_destroy_device(dev); > +#ifdef CONFIG_ATA_SYSFS_COMPAT > put_device(cdev); > +#endif > put_device(dev); > } > > @@ -288,7 +294,9 @@ int ata_tport_add(struct device *parent, > { > int error; > struct device *dev = &ap->tdev; > +#ifdef CONFIG_ATA_SYSFS_COMPAT > struct device *cdev = &ap->cdev; > +#endif > > device_initialize(dev); > dev->type = &ata_port_type; > @@ -299,23 +307,24 @@ int ata_tport_add(struct device *parent, > dev->bus = &ata_bus_type; > dev_set_name(dev, "port%d", ap->print_id); > > +#ifdef CONFIG_ATA_SYSFS_COMPAT > device_initialize(cdev); > cdev->parent = get_device(dev); > cdev->class = &ata_port_class.class; > dev_set_name(cdev, "ata%d", ap->print_id); > - > +#endif > transport_setup_device(dev); > ata_acpi_bind_port(ap); > error = device_add(dev); > if (error) { > goto tport_err; > } > - > +#ifdef CONFIG_ATA_SYSFS_COMPAT > error = device_add(cdev); > if (error) { > goto cdev_err; > } > - Instead of adding a device, can't we simply create a symlink ? E.g.: sprintf(symlink_name, "ata%d", ap->print_id); sysfs_create_link(&parent->kobj, &dev->kobj, symlink_name); ? This seems to work, but I have not checked all possible paths to see if a symlink or directory name expected to be "ataX" ends up being "portX", which could break userspace relying on the old name. > +#endif > device_enable_async_suspend(dev); > pm_runtime_set_active(dev); > pm_runtime_enable(dev); > @@ -331,14 +340,18 @@ int ata_tport_add(struct device *parent, > return 0; > > tport_link_err: > +#ifdef CONFIG_ATA_SYSFS_COMPAT > device_del(cdev); > cdev_err: > +#endif > transport_remove_device(dev); > device_del(dev); > > tport_err: > transport_destroy_device(dev); > +#ifdef CONFIG_ATA_SYSFS_COMPAT > put_device(cdev); > +#endif > put_device(dev); > ata_host_put(ap->host); > return error;
On 3/24/22 10:27, Damien Le Moal wrote: > On 3/21/22 23:56, Hannes Reinecke wrote: >> Add a config option 'ATA_SYSFS_COMPAT' to allow the user to select >> whether the compability 'ata_port' object with the name of 'ata' >> should be registered with sysfs. >> >> Signed-off-by: Hannes Reinecke <hare@suse.de> >> --- >> drivers/ata/Kconfig | 10 ++++++++++ >> drivers/ata/libata-transport.c | 19 ++++++++++++++++--- >> 2 files changed, 26 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig >> index cb54631fd950..fe42a246d21d 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 5169a5db141d..efca41039d1e 100644 >> --- a/drivers/ata/libata-transport.c >> +++ b/drivers/ata/libata-transport.c >> @@ -261,15 +261,21 @@ static int ata_tport_match(struct attribute_container *cont, >> void ata_tport_delete(struct ata_port *ap) >> { >> struct device *dev = &ap->tdev; >> +#ifdef CONFIG_ATA_SYSFS_COMPAT >> struct device *cdev = &ap->cdev; >> +#endif >> >> ata_tlink_delete(&ap->link); >> >> +#ifdef CONFIG_ATA_SYSFS_COMPAT >> device_del(cdev); >> +#endif >> transport_remove_device(dev); >> device_del(dev); >> transport_destroy_device(dev); >> +#ifdef CONFIG_ATA_SYSFS_COMPAT >> put_device(cdev); >> +#endif >> put_device(dev); >> } >> >> @@ -288,7 +294,9 @@ int ata_tport_add(struct device *parent, >> { >> int error; >> struct device *dev = &ap->tdev; >> +#ifdef CONFIG_ATA_SYSFS_COMPAT >> struct device *cdev = &ap->cdev; >> +#endif >> >> device_initialize(dev); >> dev->type = &ata_port_type; >> @@ -299,23 +307,24 @@ int ata_tport_add(struct device *parent, >> dev->bus = &ata_bus_type; >> dev_set_name(dev, "port%d", ap->print_id); >> >> +#ifdef CONFIG_ATA_SYSFS_COMPAT >> device_initialize(cdev); >> cdev->parent = get_device(dev); >> cdev->class = &ata_port_class.class; >> dev_set_name(cdev, "ata%d", ap->print_id); >> - >> +#endif >> transport_setup_device(dev); >> ata_acpi_bind_port(ap); >> error = device_add(dev); >> if (error) { >> goto tport_err; >> } >> - >> +#ifdef CONFIG_ATA_SYSFS_COMPAT >> error = device_add(cdev); >> if (error) { >> goto cdev_err; >> } >> - > > Instead of adding a device, can't we simply create a symlink ? > E.g.: > > sprintf(symlink_name, "ata%d", ap->print_id); > sysfs_create_link(&parent->kobj, &dev->kobj, symlink_name); > > ? This seems to work, but I have not checked all possible paths to see if > a symlink or directory name expected to be "ataX" ends up being "portX", > which could break userspace relying on the old name. > Yeah; tried a similar thing which turned out not to be working. Let me test your patch. Cheers, Hannes
diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig index cb54631fd950..fe42a246d21d 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 5169a5db141d..efca41039d1e 100644 --- a/drivers/ata/libata-transport.c +++ b/drivers/ata/libata-transport.c @@ -261,15 +261,21 @@ static int ata_tport_match(struct attribute_container *cont, void ata_tport_delete(struct ata_port *ap) { struct device *dev = &ap->tdev; +#ifdef CONFIG_ATA_SYSFS_COMPAT struct device *cdev = &ap->cdev; +#endif ata_tlink_delete(&ap->link); +#ifdef CONFIG_ATA_SYSFS_COMPAT device_del(cdev); +#endif transport_remove_device(dev); device_del(dev); transport_destroy_device(dev); +#ifdef CONFIG_ATA_SYSFS_COMPAT put_device(cdev); +#endif put_device(dev); } @@ -288,7 +294,9 @@ int ata_tport_add(struct device *parent, { int error; struct device *dev = &ap->tdev; +#ifdef CONFIG_ATA_SYSFS_COMPAT struct device *cdev = &ap->cdev; +#endif device_initialize(dev); dev->type = &ata_port_type; @@ -299,23 +307,24 @@ int ata_tport_add(struct device *parent, dev->bus = &ata_bus_type; dev_set_name(dev, "port%d", ap->print_id); +#ifdef CONFIG_ATA_SYSFS_COMPAT device_initialize(cdev); cdev->parent = get_device(dev); cdev->class = &ata_port_class.class; dev_set_name(cdev, "ata%d", ap->print_id); - +#endif transport_setup_device(dev); ata_acpi_bind_port(ap); error = device_add(dev); if (error) { goto tport_err; } - +#ifdef CONFIG_ATA_SYSFS_COMPAT error = device_add(cdev); if (error) { goto cdev_err; } - +#endif device_enable_async_suspend(dev); pm_runtime_set_active(dev); pm_runtime_enable(dev); @@ -331,14 +340,18 @@ int ata_tport_add(struct device *parent, return 0; tport_link_err: +#ifdef CONFIG_ATA_SYSFS_COMPAT device_del(cdev); cdev_err: +#endif transport_remove_device(dev); device_del(dev); tport_err: transport_destroy_device(dev); +#ifdef CONFIG_ATA_SYSFS_COMPAT put_device(cdev); +#endif put_device(dev); ata_host_put(ap->host); return error;
Add a config option 'ATA_SYSFS_COMPAT' to allow the user to select whether the compability 'ata_port' object with the name of 'ata' should be registered with sysfs. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/ata/Kconfig | 10 ++++++++++ drivers/ata/libata-transport.c | 19 ++++++++++++++++--- 2 files changed, 26 insertions(+), 3 deletions(-)