Hi Bartlomiej,
On 6/24/20 5:35 AM, Bartlomiej Zolnierkiewicz wrote:
> The root source of problem is that libata transport uses different
> naming scheme for ->tdev devices (please see dev_set_name() in
> ata_t{dev,link,port}_add()) than libata core for its logging
> functionality (ata_{dev,link,port}_printk()).
>
> Since libata transport is part of sysfs ABI we should be careful
> to not break it so one idea for solving the issue is to convert
> ata_t{dev,link,port}_add() to use libata logging naming scheme and
> at the same time add sysfs symlinks for the old libata transport
> naming scheme.
I tried doing as you suggested. I've included what I've done so far. I
haven't been able to get all the needed parts for the symlinks to
maintain compatibility.
The /sys/class/.. seems OK, eg.
$ ls -x -w 70 /sys/class/ata_[dl]*
/sys/class/ata_device:
ata1.00 ata2.00 ata3.00 ata4.00 ata5.00 ata6.00 ata7.00
ata7.01 ata8.00 ata8.01 dev1.0 dev2.0 dev3.0 dev4.0
dev5.0 dev6.0 dev7.0 dev7.1 dev8.0 dev8.1
/sys/class/ata_link:
ata1 ata2 ata3 ata4 ata5 ata6 ata7 ata8 link1 link2
link3 link4 link5 link6 link7 link8
but the implementation is a hack, see device.h, core.c changes. There
must be a better way?
Also I'm missing part of the full path, eg.
/sys/devices/pci0000:00/0000:00:01.1/ata7/link7/dev7.0/ata_device/dev7.0/gscr
becomes
/sys/devices/pci0000:00/0000:00:01.1/ata7/ata7/ata7.01/ata_device/ata7.01/gscr
but the compatibility symlinks added only get me to
/sys/devices/pci0000:00/0000:00:01.1/ata7/link7/dev7.0/ata_device/
I haven't found the right spot to get the last symlink included.
If you or anyone else has suggestions to correct the incomplete symlink
and/or correct the implementation to set the
/sys/class/ata_device it would be greatly appreciated.
Thanks,
-Tony
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index beca5f91bb4c..2aa230ad813e 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6444,7 +6444,7 @@ void ata_port_printk(const struct ata_port *ap,
const char *level,
vaf.fmt = fmt;
vaf.va = &args;
- printk("%sata%u: %pV", level, ap->print_id, &vaf);
+ dev_printk(level, &ap->tdev, "%pV", &vaf);
va_end(args);
}
@@ -6461,12 +6461,7 @@ void ata_link_printk(const struct ata_link *link,
const char *level,
vaf.fmt = fmt;
vaf.va = &args;
- if (sata_pmp_attached(link->ap) || link->ap->slave_link)
- printk("%sata%u.%02u: %pV",
- level, link->ap->print_id, link->pmp, &vaf);
- else
- printk("%sata%u: %pV",
- level, link->ap->print_id, &vaf);
+ dev_printk(level, &link->tdev, "%pV", &vaf);
va_end(args);
}
@@ -6483,9 +6478,7 @@ void ata_dev_printk(const struct ata_device *dev,
const char *level,
vaf.fmt = fmt;
vaf.va = &args;
- printk("%sata%u.%02u: %pV",
- level, dev->link->ap->print_id, dev->link->pmp + dev->devno,
- &vaf);
+ dev_printk(level, &dev->tdev,"%pV", &vaf);
va_end(args);
}
diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index 6a40e3c6cf49..e5870f9b2b21 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -288,7 +288,7 @@ 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_set_name(dev, "ata%u", ap->print_id);
transport_setup_device(dev);
ata_acpi_bind_port(ap);
error = device_add(dev);
@@ -374,6 +374,46 @@ static int ata_tlink_match(struct
attribute_container *cont,
return &i->link_attr_cont.ac == cont;
}
+static struct device* tlink_to_symlink(struct device *dev) {
+ struct ata_internal* i =
to_ata_internal(ata_scsi_transport_template);
+ return
attribute_container_find_class_device(&i->link_attr_cont.ac, dev);
+}
+
+void ata_tlink_symlink_add_del(struct ata_link *link, int add) {
+ struct device *dev = &link->tdev;
+ struct ata_port *ap = link->ap;
+ char lname[64];
+ struct device *devp = tlink_to_symlink(dev);
+
+ if (ata_is_host_link(link)) {
+ snprintf(lname, sizeof(lname),
+ "link%d", ap->print_id);
+ } else {
+ snprintf(lname, sizeof(lname),
+ "link%d.%d", ap->print_id, link->pmp);
+ }
+
+ if (add) {
+ int e;
+
+ e = device_add_class_symlinks_additional(devp, lname);
+ if (e) {
+ dev_warn(devp, "Error %d tlink symlink
class\n", e);
+ }
+
+ e = sysfs_create_link(&dev->parent->kobj, &dev->kobj,
lname);
+ if (e) {
+ dev_warn(dev, "Error %d tlink symlink\n", e);
+ }
+ } else {
+ sysfs_delete_link(&dev->parent->kobj, &dev->kobj, lname);
+ device_delete_class_symlinks_additional(devp, lname);
+ }
+}
+
+#define ata_tlink_symlink_add(x) ata_tlink_symlink_add_del(x, 1)
+#define ata_tlink_symlink_del(x) ata_tlink_symlink_add_del(x, 0)
+
/**
* ata_tlink_delete -- remove ATA LINK
* @port: ATA LINK to remove
@@ -389,6 +429,7 @@ void ata_tlink_delete(struct ata_link *link)
ata_tdev_delete(ata_dev);
}
+ ata_tlink_symlink_del(link);
transport_remove_device(dev);
device_del(dev);
transport_destroy_device(dev);
@@ -415,9 +456,9 @@ int ata_tlink_add(struct ata_link *link)
dev->parent = &ap->tdev;
dev->release = ata_tlink_release;
if (ata_is_host_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_set_name(dev, "ata%u", ap->print_id);
+ else
+ dev_set_name(dev, "ata%u.%02u", ap->print_id, link->pmp);
transport_setup_device(dev);
@@ -429,6 +470,8 @@ int ata_tlink_add(struct ata_link *link)
transport_add_device(dev);
transport_configure_device(dev);
+ ata_tlink_symlink_add(link);
+
ata_for_each_dev(ata_dev, link, ALL) {
error = ata_tdev_add(ata_dev);
if (error) {
@@ -440,6 +483,7 @@ int ata_tlink_add(struct ata_link *link)
while (--ata_dev >= link->device) {
ata_tdev_delete(ata_dev);
}
+ ata_tlink_symlink_del(link);
transport_remove_device(dev);
device_del(dev);
tlink_err:
@@ -630,6 +674,44 @@ static void ata_tdev_free(struct ata_device *dev)
put_device(&dev->tdev);
}
+static struct device* tdev_to_symlink(struct device *dev) {
+ struct ata_internal* i =
to_ata_internal(ata_scsi_transport_template);
+ return
attribute_container_find_class_device(&i->dev_attr_cont.ac, dev);
+}
+
+void ata_tdev_symlink_add_del(struct ata_device *ata_dev, int add) {
+ struct device *dev = &ata_dev->tdev;
+ struct ata_link *link = ata_dev->link;
+ struct ata_port *ap = link->ap;
+ char dname[64];
+
+ struct device *devp = tdev_to_symlink(dev);
+
+ if (ata_is_host_link(link))
+ snprintf(dname, sizeof(dname), "dev%d.%d",
ap->print_id,ata_dev->devno);
+ else
+ snprintf(dname, sizeof(dname), "dev%d.%d.0",
ap->print_id, link->pmp);
+
+ if (add) {
+ int e;
+ e = device_add_class_symlinks_additional(devp, dname);
+ if (e) {
+ dev_warn(devp, "Error %d tdev symlink class\n", e);
+ }
+
+ e = sysfs_create_link(&dev->parent->kobj, &dev->kobj,
dname);
+ if (e) {
+ dev_warn(dev, "Error %d tdev symlink\n", e);
+ }
+ } else {
+ sysfs_delete_link(&dev->parent->kobj, &dev->kobj, dname);
+ device_delete_class_symlinks_additional(devp, dname);
+ }
+}
+
+#define ata_tdev_symlink_add(x) ata_tdev_symlink_add_del(x, 1)
+#define ata_tdev_symlink_del(x) ata_tdev_symlink_add_del(x, 0)
+
/**
* ata_tdev_delete -- remove ATA device
* @port: ATA PORT to remove
@@ -640,6 +722,7 @@ static void ata_tdev_delete(struct ata_device *ata_dev)
{
struct device *dev = &ata_dev->tdev;
+ ata_tdev_symlink_del(ata_dev);
transport_remove_device(dev);
device_del(dev);
ata_tdev_free(ata_dev);
@@ -665,10 +748,8 @@ static int ata_tdev_add(struct ata_device *ata_dev)
device_initialize(dev);
dev->parent = &link->tdev;
dev->release = ata_tdev_release;
- if (ata_is_host_link(link))
- 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_set_name(dev, "ata%u.%02u", ap->print_id, link->pmp +
ata_dev->devno);
transport_setup_device(dev);
ata_acpi_bind_dev(ata_dev);
@@ -680,6 +761,8 @@ static int ata_tdev_add(struct ata_device *ata_dev)
transport_add_device(dev);
transport_configure_device(dev);
+
+ ata_tdev_symlink_add(ata_dev);
return 0;
}
diff --git a/drivers/base/core.c b/drivers/base/core.c
index c2439d12608d..bc060000837c 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2269,6 +2269,16 @@ static int device_add_class_symlinks(struct
device *dev)
return error;
}
+int device_add_class_symlinks_additional(struct device *dev, char *name) {
+ return sysfs_create_link(&dev->class->p->subsys.kobj,
&dev->kobj, name);
+}
+EXPORT_SYMBOL_GPL(device_add_class_symlinks_additional);
+
+void device_delete_class_symlinks_additional(struct device *dev, char
*name) {
+ sysfs_delete_link(&dev->class->p->subsys.kobj, &dev->kobj, name);
+}
+EXPORT_SYMBOL_GPL(device_delete_class_symlinks_additional);
+
static void device_remove_class_symlinks(struct device *dev)
{
if (dev_of_node(dev))
diff --git a/include/linux/device.h b/include/linux/device.h
index 281755404c21..4827d86116ab 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -656,6 +656,10 @@ static inline const char *dev_name(const struct
device *dev)
extern __printf(2, 3)
int dev_set_name(struct device *dev, const char *name, ...);
+
+int device_add_class_symlinks_additional(struct device *dev, char *name);
+void device_delete_class_symlinks_additional(struct device *dev, char
*name);
+
int dev_durable_name(const struct device *d, char *buffer, size_t len);
#ifdef CONFIG_NUMA
