| Message ID | 20220817150542.483291-7-nipun.gupta@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add support for CDX bus controller | expand |
On Wed, Aug 17, 2022 at 08:35:42PM +0530, Nipun Gupta wrote: > This change adds compatible string for the platform based > devices. What exactly is a "compatible string"? > > Signed-off-by: Nipun Gupta <nipun.gupta@amd.com> > --- > Documentation/ABI/testing/sysfs-bus-platform | 8 +++++++ > drivers/base/platform.c | 23 ++++++++++++++++++++ > 2 files changed, 31 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-platform b/Documentation/ABI/testing/sysfs-bus-platform > index c4dfe7355c2d..d95ff83d768c 100644 > --- a/Documentation/ABI/testing/sysfs-bus-platform > +++ b/Documentation/ABI/testing/sysfs-bus-platform > @@ -54,3 +54,11 @@ Description: > Other platform devices use, instead: > > - platform:`driver name` > + > +What: /sys/bus/platform/devices/.../compatible > +Date: August 2022 > +Contact: Nipun Gupta <nipun.gupta@amd.com> > +Description: > + compatible string associated with the device. This is > + a read only and is visible if the device have "compatible" > + property associated with it. Where is it defined what a compatible property is? > diff --git a/drivers/base/platform.c b/drivers/base/platform.c > index 51bb2289865c..94c33efaa9b8 100644 > --- a/drivers/base/platform.c > +++ b/drivers/base/platform.c > @@ -1289,10 +1289,25 @@ static ssize_t driver_override_store(struct device *dev, > } > static DEVICE_ATTR_RW(driver_override); > > +static ssize_t compatible_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + const char *compat; > + int ret; > + > + ret = device_property_read_string(dev, "compatible", &compat); > + if (ret != 0) > + return 0; Shouldn't you return an error here? thanks, greg k-h
On Wed, Aug 17, 2022 at 8:31 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Wed, Aug 17, 2022 at 08:35:42PM +0530, Nipun Gupta wrote: > > This change adds compatible string for the platform based > > devices. > > What exactly is a "compatible string"? Didn't read the rest of the patches in the series yet, but Nack to this. This info is already available under: <device folder>/of_node/compatible for any device in any (or at least most) bus that was created from an of_node. Unless compatible is now also in ACPI. In which case, it's probably be better to have an of_node like symlink. -Saravana > > > > > Signed-off-by: Nipun Gupta <nipun.gupta@amd.com> > > --- > > Documentation/ABI/testing/sysfs-bus-platform | 8 +++++++ > > drivers/base/platform.c | 23 ++++++++++++++++++++ > > 2 files changed, 31 insertions(+) > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-platform b/Documentation/ABI/testing/sysfs-bus-platform > > index c4dfe7355c2d..d95ff83d768c 100644 > > --- a/Documentation/ABI/testing/sysfs-bus-platform > > +++ b/Documentation/ABI/testing/sysfs-bus-platform > > @@ -54,3 +54,11 @@ Description: > > Other platform devices use, instead: > > > > - platform:`driver name` > > + > > +What: /sys/bus/platform/devices/.../compatible > > +Date: August 2022 > > +Contact: Nipun Gupta <nipun.gupta@amd.com> > > +Description: > > + compatible string associated with the device. This is > > + a read only and is visible if the device have "compatible" > > + property associated with it. > > Where is it defined what a compatible property is? > > > diff --git a/drivers/base/platform.c b/drivers/base/platform.c > > index 51bb2289865c..94c33efaa9b8 100644 > > --- a/drivers/base/platform.c > > +++ b/drivers/base/platform.c > > @@ -1289,10 +1289,25 @@ static ssize_t driver_override_store(struct device *dev, > > } > > static DEVICE_ATTR_RW(driver_override); > > > > +static ssize_t compatible_show(struct device *dev, struct device_attribute *attr, > > + char *buf) > > +{ > > + const char *compat; > > + int ret; > > + > > + ret = device_property_read_string(dev, "compatible", &compat); > > + if (ret != 0) > > + return 0; > > Shouldn't you return an error here? > > thanks, > > greg k-h
[AMD Official Use Only - General] > -----Original Message----- > From: Saravana Kannan <saravanak@google.com> > Sent: Wednesday, August 17, 2022 9:34 PM > To: Greg KH <gregkh@linuxfoundation.org> > Cc: Gupta, Nipun <Nipun.Gupta@amd.com>; robh+dt@kernel.org; > krzysztof.kozlowski+dt@linaro.org; rafael@kernel.org; eric.auger@redhat.com; > alex.williamson@redhat.com; cohuck@redhat.com; Gupta, Puneet (DCG-ENG) > <puneet.gupta@amd.com>; song.bao.hua@hisilicon.com; > mchehab+huawei@kernel.org; maz@kernel.org; f.fainelli@gmail.com; > jeffrey.l.hugo@gmail.com; Michael.Srba@seznam.cz; mani@kernel.org; > yishaih@nvidia.com; jgg@ziepe.ca; linux-kernel@vger.kernel.org; > devicetree@vger.kernel.org; kvm@vger.kernel.org; okaya@kernel.org; Anand, > Harpreet <harpreet.anand@amd.com>; Agarwal, Nikhil > <nikhil.agarwal@amd.com>; Simek, Michal <michal.simek@amd.com>; git > (AMD-Xilinx) <git@amd.com> > Subject: Re: [RFC PATCH v2 6/6] driver core: add compatible string in sysfs for > platform devices > > [CAUTION: External Email] > > On Wed, Aug 17, 2022 at 8:31 AM Greg KH <gregkh@linuxfoundation.org> > wrote: > > > > On Wed, Aug 17, 2022 at 08:35:42PM +0530, Nipun Gupta wrote: > > > This change adds compatible string for the platform based > > > devices. > > > > What exactly is a "compatible string"? > > Didn't read the rest of the patches in the series yet, but Nack to > this. This info is already available under: > > <device folder>/of_node/compatible for any device in any (or at least > most) bus that was created from an of_node. > > Unless compatible is now also in ACPI. In which case, it's probably be > better to have an of_node like symlink. We will not be going with platform devices for CDX bus and would rather have CDX devices: https://lore.kernel.org/lkml/DM6PR12MB30827577D50AB1B877458923E8799@DM6PR12MB3082.namprd12.prod.outlook.com/ So, this change is not valid for us. We would not be having this in Rev V3. - Nipun
diff --git a/Documentation/ABI/testing/sysfs-bus-platform b/Documentation/ABI/testing/sysfs-bus-platform index c4dfe7355c2d..d95ff83d768c 100644 --- a/Documentation/ABI/testing/sysfs-bus-platform +++ b/Documentation/ABI/testing/sysfs-bus-platform @@ -54,3 +54,11 @@ Description: Other platform devices use, instead: - platform:`driver name` + +What: /sys/bus/platform/devices/.../compatible +Date: August 2022 +Contact: Nipun Gupta <nipun.gupta@amd.com> +Description: + compatible string associated with the device. This is + a read only and is visible if the device have "compatible" + property associated with it. diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 51bb2289865c..94c33efaa9b8 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -1289,10 +1289,25 @@ static ssize_t driver_override_store(struct device *dev, } static DEVICE_ATTR_RW(driver_override); +static ssize_t compatible_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + const char *compat; + int ret; + + ret = device_property_read_string(dev, "compatible", &compat); + if (ret != 0) + return 0; + + return sysfs_emit(buf, "%s", compat); +} +static DEVICE_ATTR_RO(compatible); + static struct attribute *platform_dev_attrs[] = { &dev_attr_modalias.attr, &dev_attr_numa_node.attr, &dev_attr_driver_override.attr, + &dev_attr_compatible.attr, NULL, }; @@ -1300,11 +1315,19 @@ static umode_t platform_dev_attrs_visible(struct kobject *kobj, struct attribute int n) { struct device *dev = container_of(kobj, typeof(*dev), kobj); + const char *compat; + int ret; if (a == &dev_attr_numa_node.attr && dev_to_node(dev) == NUMA_NO_NODE) return 0; + if (a == &dev_attr_compatible.attr) { + ret = device_property_read_string(dev, "compatible", &compat); + if (ret != 0) + return 0; + } + return a->mode; }
This change adds compatible string for the platform based devices. Signed-off-by: Nipun Gupta <nipun.gupta@amd.com> --- Documentation/ABI/testing/sysfs-bus-platform | 8 +++++++ drivers/base/platform.c | 23 ++++++++++++++++++++ 2 files changed, 31 insertions(+)