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@xxxxxxx>
> ---
> 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;
--
Damien Le Moal
Western Digital Research
