[permalink] [raw]

Subject: [PATCH 11/16] powerpc: vio_cmo: use dev_groups and not dev_attrs for bus_type

The dev_attrs field has long been "depreciated" and is finally being
removed, so move the driver to use the "correct" dev_groups field
instead for struct bus_type.

Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Vineet Gupta <[email protected]>
Cc: Bart Van Assche <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Johan Hovold <[email protected]>
Cc: Alexey Kardashevskiy <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
arch/powerpc/platforms/pseries/vio.c | 37 +++++++++++++++++++++---------------
1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
index fd6595598662..176edf422867 100644
--- a/arch/powerpc/platforms/pseries/vio.c
+++ b/arch/powerpc/platforms/pseries/vio.c
@@ -948,7 +948,7 @@ static void vio_cmo_bus_init(void)
/* sysfs device functions and data structures for CMO */

#define viodev_cmo_rd_attr(name) \
-static ssize_t viodev_cmo_##name##_show(struct device *dev, \
+static ssize_t cmo_##name##_show(struct device *dev, \
struct device_attribute *attr, \
char *buf) \
{ \
@@ -962,7 +962,7 @@ static ssize_t viodev_cmo_allocs_failed_show(struct device *dev,
return sprintf(buf, "%d\n", atomic_read(&viodev->cmo.allocs_failed));
}

-static ssize_t viodev_cmo_allocs_failed_reset(struct device *dev,
+static ssize_t cmo_allocs_failed_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count)
{
struct vio_dev *viodev = to_vio_dev(dev);
@@ -970,7 +970,7 @@ static ssize_t viodev_cmo_allocs_failed_reset(struct device *dev,
return count;
}

-static ssize_t viodev_cmo_desired_set(struct device *dev,
+static ssize_t cmo_desired_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count)
{
struct vio_dev *viodev = to_vio_dev(dev);
@@ -993,18 +993,25 @@ static ssize_t name_show(struct device *, struct device_attribute *, char *);
static ssize_t devspec_show(struct device *, struct device_attribute *, char *);
static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
char *buf);
-static struct device_attribute vio_cmo_dev_attrs[] = {
- __ATTR_RO(name),
- __ATTR_RO(devspec),
- __ATTR_RO(modalias),
- __ATTR(cmo_desired, S_IWUSR|S_IRUSR|S_IWGRP|S_IRGRP|S_IROTH,
- viodev_cmo_desired_show, viodev_cmo_desired_set),
- __ATTR(cmo_entitled, S_IRUGO, viodev_cmo_entitled_show, NULL),
- __ATTR(cmo_allocated, S_IRUGO, viodev_cmo_allocated_show, NULL),
- __ATTR(cmo_allocs_failed, S_IWUSR|S_IRUSR|S_IWGRP|S_IRGRP|S_IROTH,
- viodev_cmo_allocs_failed_show, viodev_cmo_allocs_failed_reset),
- __ATTR_NULL
+static DEVICE_ATTR_RO(name);
+static DEVICE_ATTR_RO(devspec);
+static DEVICE_ATTR_RO(modalias);
+static DEVICE_ATTR_RO(cmo_entitled);
+static DEVICE_ATTR_RO(cmo_allocated);
+static DEVICE_ATTR_RW(cmo_desired);
+static DEVICE_ATTR_RW(cmo_allocs_failed);
+
+static struct attribute *vio_cmo_dev_attrs[] = {
+ &dev_attr_name.attr,
+ &dev_attr_devspec.attr,
+ &dev_attr_modalias.attr,
+ &dev_attr_cmo_entitled.attr,
+ &dev_attr_cmo_allocated.attr,
+ &dev_attr_cmo_desired.attr,
+ &dev_attr_cmo_allocs_failed.attr,
+ NULL,
};
+ATTRIBUTE_GROUPS(vio_cmo_dev);

/* sysfs bus functions and data structures for CMO */

@@ -1066,7 +1073,7 @@ ATTRIBUTE_GROUPS(vio_bus);

static void vio_cmo_sysfs_init(void)
{
- vio_bus_type.dev_attrs = vio_cmo_dev_attrs;
+ vio_bus_type.dev_groups = vio_cmo_dev_groups;
vio_bus_type.bus_groups = vio_bus_groups;
}
#else /* CONFIG_PPC_SMLPAR */
--
2.13.0

2017-06-06 19:24:41

by Greg Kroah-Hartman

[permalink] [raw]

Greg Kroah-Hartman <[email protected]> writes:

> On Fri, Jun 09, 2017 at 09:23:10PM +1000, Michael Ellerman wrote:
>> Greg Kroah-Hartman <[email protected]> writes:
>>
>> > On Fri, Jun 09, 2017 at 08:53:22AM +1000, Michael Ellerman wrote:
>> >> Greg Kroah-Hartman <[email protected]> writes:
>> >>
>> >> > On Thu, Jun 08, 2017 at 11:12:10PM +1000, Michael Ellerman wrote:
>> >> >> Greg Kroah-Hartman <[email protected]> writes:
>> >> >>
>> >> >> > The dev_attrs field has long been "depreciated" and is finally being
>> >> >> > removed, so move the driver to use the "correct" dev_groups field
>> >> >> > instead for struct bus_type.
>> >> >> >
>> >> >> > Cc: Benjamin Herrenschmidt <[email protected]>
>> >> >> > Cc: Paul Mackerras <[email protected]>
>> >> >> > Cc: Michael Ellerman <[email protected]>
>> >> >> > Cc: Vineet Gupta <[email protected]>
>> >> >> > Cc: Bart Van Assche <[email protected]>
>> >> >> > Cc: Robin Murphy <[email protected]>
>> >> >> > Cc: Joerg Roedel <[email protected]>
>> >> >> > Cc: Johan Hovold <[email protected]>
>> >> >> > Cc: Alexey Kardashevskiy <[email protected]>
>> >> >> > Cc: Krzysztof Kozlowski <[email protected]>
>> >> >> > Cc: <[email protected]>
>> >> >> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
>> >> >> > ---
>> >> >> > arch/powerpc/platforms/pseries/vio.c | 37 +++++++++++++++++++++---------------
>> >> >> > 1 file changed, 22 insertions(+), 15 deletions(-)
>> >> >>
>> >> >> This one needed a bit more work to get building, the incremental diff is
>> >> >> below. We need a forward declaration of name, devspec and modalias,
>> >> >> which is a bit weird, but that's how the code is currently structured.
>> >> >> And there's dev and bus attributes with the same name, so that needed an
>> >> >> added "bus".
>> >> >>
>> >> >> I booted v2 of patch 10 and this one and everything looks identical to
>> >> >> upstream.
>> >> >
>> >> > Ah, many thanks, this was on my todo list to fix up today.
>> >> >
>> >> > But you renamed the sysfs files when you added "bus" to the function
>> >> > names, are you sure you want to do that? I don't mind, but if you
>> >> > happen to have userspace tools that look at those files, they just broke
>> >> > :(
>> >>
>> >> Ugh crap, no that won't work.
>> >>
>> >> I didn't see it when I tested because my machine doesn't have the CMO
>> >> feature enabled.
>> >>
>> >> I guess we have to open code some of the BUS_ATTR_RO() etc. so we can
>> >> avoid the name clash.
>> >
>> > Or split it into multiple files, I've solved this that way in the past.
>> > You shouldn't have to "open code" BUS_ATTR_RO().
>>
>> It just requires one use of __ATTR(), which seems simpler than splitting
>> the file in two.
>>
>> Here's a new incremental diff against your patch.
>>
>> I confirmed none of the cmo names changed, result after is:
>>
>> ./devices/vio/cmo_desired
>> ./devices/vio/cmo_allocated
>> ./devices/vio/cmo_entitled
>> ./devices/vio/cmo_allocs_failed
>> ./devices/vio/71000000/cmo_desired
>> ./devices/vio/71000000/cmo_allocated
>> ./devices/vio/71000000/cmo_entitled
>> ./devices/vio/71000000/cmo_allocs_failed
>> ./devices/vio/30000000/cmo_desired
>> ./devices/vio/30000000/cmo_allocated
>> ./devices/vio/30000000/cmo_entitled
>> ./devices/vio/30000000/cmo_allocs_failed
>> ./devices/vio/2000/cmo_desired
>> ./devices/vio/2000/cmo_allocated
>> ./devices/vio/2000/cmo_entitled
>> ./devices/vio/2000/cmo_allocs_failed
>> ./bus/vio/cmo_high
>> ./bus/vio/cmo_spare
>> ./bus/vio/cmo_reserve_size
>> ./bus/vio/cmo_desired
>> ./bus/vio/cmo_entitled
>> ./bus/vio/cmo_excess_free
>> ./bus/vio/cmo_excess_size
>> ./bus/vio/cmo_min
>> ./bus/vio/cmo_curr
>
> Thanks for this, it seems to have passed all of the 0-day testing. I'll
> go apply it to my "real" tree now, thanks again for the help.

No worries. It'll get some more build & boot testing from my CI once it's
in linux-next.

cheers