Re: [PATCH v3 4/4] bus: fsl-mc: add bus rescan attribute
From: Greg KH
Date: Tue Apr 03 2018 - 03:05:04 EST
On Mon, Apr 02, 2018 at 01:46:13PM +0000, Ioana Ciornei wrote:
> > > Introduce the rescan attribute as a bus attribute to synchronize the
> > > fsl-mc bus objects and the MC firmware.
> > >
> > > To rescan the fsl-mc bus, e.g.,
> > > echo 1 > /sys/bus/fsl-mc/rescan
> > >
> > > Signed-off-by: Ioana Ciornei <ioana.ciornei@xxxxxxx>
> > > ---
> > > Changes in v2:
> > > - added proper documentation in /Documentation/ABI/
> > > - updated the MAINTAINERS file
> > > Changes in v3:
> > > - no change
> > >
> > > Documentation/ABI/stable/sysfs-bus-fsl-mc | 7 +++++
> > > drivers/bus/fsl-mc/fsl-mc-bus.c | 48
> > +++++++++++++++++++++++++++++++
> > > 2 files changed, 55 insertions(+)
> > >
> > > diff --git a/Documentation/ABI/stable/sysfs-bus-fsl-mc
> > > b/Documentation/ABI/stable/sysfs-bus-fsl-mc
> > > index e530e8c..0663fbd 100644
> > > --- a/Documentation/ABI/stable/sysfs-bus-fsl-mc
> > > +++ b/Documentation/ABI/stable/sysfs-bus-fsl-mc
> > > @@ -4,3 +4,10 @@ KernelVersion: 4.16
> > > Contact: Ioana Ciornei <ioana.ciornei@xxxxxxx>
> > > Description: Root dprc rescan attribute
> > > Users: Userspace drivers and management tools
> > > +
> > > +What: /sys/bus/fsl-mc/rescan
> > > +Date: March. 2018
> > > +KernelVersion: 4.16
> >
> > Same comments as previous review.
> >
> > > +Contact: Ioana Ciornei <ioana.ciornei@xxxxxxx>
> > > +Description: Bus rescan attribute
> >
> > Again, describe this better please.
> >
> > > +Users: Userspace drivers and management tools
> > > diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c
> > > b/drivers/bus/fsl-mc/fsl-mc-bus.c index 9d02984..80010d1 100644
> > > --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> > > +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> > > @@ -172,11 +172,59 @@ static ssize_t rescan_store(struct device *dev,
> > >
> > > ATTRIBUTE_GROUPS(fsl_mc_dev);
> > >
> > > +static int scan_fsl_mc_bus(struct device *dev, void *data) {
> > > + struct fsl_mc_device *root_mc_dev;
> > > + struct fsl_mc_bus *root_mc_bus;
> > > +
> > > + if (!fsl_mc_is_root_dprc(dev))
> > > + goto exit;
> > > +
> > > + root_mc_dev = to_fsl_mc_device(dev);
> > > + root_mc_bus = to_fsl_mc_bus(root_mc_dev);
> > > + mutex_lock(&root_mc_bus->scan_mutex);
> > > + dprc_scan_objects(root_mc_dev, NULL);
> > > + mutex_unlock(&root_mc_bus->scan_mutex);
> > > +
> > > +exit:
> > > + return 0;
> > > +}
> > > +
> > > +static ssize_t bus_rescan_store(struct bus_type *bus,
> > > + const char *buf, size_t count)
> > > +{
> > > + unsigned long val;
> > > +
> > > + if (kstrtoul(buf, 0, &val) < 0)
> > > + return -EINVAL;
> > > +
> > > + if (val)
> > > + bus_for_each_dev(bus, NULL, NULL, scan_fsl_mc_bus);
> > > +
> > > + return count;
> > > +}
> > > +static BUS_ATTR(rescan, 0220, NULL, bus_rescan_store);
> >
> > BUS_ATTR_RO()?
> >
> >
>
> Since this is a write-only attribute, a BUS_ATTR_WO would be needed but there is no WO macro defined.
Oops, yes. We can easily fix that :)
thanks,
greg k-h