* [PATCH] usb: typec: Send uevent for num_altmodes update @ 2021-01-07 3:49 Prashant Malani 2021-01-07 9:17 ` Greg KH 0 siblings, 1 reply; 6+ messages in thread From: Prashant Malani @ 2021-01-07 3:49 UTC (permalink / raw) To: linux-usb, gregkh Cc: Prashant Malani, Heikki Krogerus, Benson Leung, open list Generate a change uevent when the "number_of_alternate_modes" sysfs file for partners and plugs is updated by a port driver. Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> Cc: Benson Leung <bleung@chromium.org> Signed-off-by: Prashant Malani <pmalani@chromium.org> --- drivers/usb/typec/class.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c index ebfd3113a9a8..8f77669f9cf4 100644 --- a/drivers/usb/typec/class.c +++ b/drivers/usb/typec/class.c @@ -766,6 +766,7 @@ int typec_partner_set_num_altmodes(struct typec_partner *partner, int num_altmod return ret; sysfs_notify(&partner->dev.kobj, NULL, "number_of_alternate_modes"); + kobject_uevent(&partner->dev.kobj, KOBJ_CHANGE); return 0; } @@ -923,6 +924,7 @@ int typec_plug_set_num_altmodes(struct typec_plug *plug, int num_altmodes) return ret; sysfs_notify(&plug->dev.kobj, NULL, "number_of_alternate_modes"); + kobject_uevent(&plug->dev.kobj, KOBJ_CHANGE); return 0; } -- 2.29.2.729.g45daf8777d-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] usb: typec: Send uevent for num_altmodes update 2021-01-07 3:49 [PATCH] usb: typec: Send uevent for num_altmodes update Prashant Malani @ 2021-01-07 9:17 ` Greg KH 2021-01-07 9:50 ` Prashant Malani 0 siblings, 1 reply; 6+ messages in thread From: Greg KH @ 2021-01-07 9:17 UTC (permalink / raw) To: Prashant Malani; +Cc: linux-usb, Heikki Krogerus, Benson Leung, open list On Wed, Jan 06, 2021 at 07:49:04PM -0800, Prashant Malani wrote: > Generate a change uevent when the "number_of_alternate_modes" sysfs file > for partners and plugs is updated by a port driver. > > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Cc: Benson Leung <bleung@chromium.org> > Signed-off-by: Prashant Malani <pmalani@chromium.org> > --- > drivers/usb/typec/class.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c > index ebfd3113a9a8..8f77669f9cf4 100644 > --- a/drivers/usb/typec/class.c > +++ b/drivers/usb/typec/class.c > @@ -766,6 +766,7 @@ int typec_partner_set_num_altmodes(struct typec_partner *partner, int num_altmod > return ret; > > sysfs_notify(&partner->dev.kobj, NULL, "number_of_alternate_modes"); > + kobject_uevent(&partner->dev.kobj, KOBJ_CHANGE); Shouldn't the sysfs_notify() handle the "something has changed" logic good enough for userspace, as obviously someone is polling on the thing (otherwise we wouldn't be calling sysfs_notify...) The kobject itself hasn't "changed", but rather an individual attribute has changed. We don't want to create uevents for every individual sysfs attribute changing values, do we? What is preventing a normal "monitor the sysfs file" logic from working here for anyone who wants to know that the alternate modes have changed? thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] usb: typec: Send uevent for num_altmodes update 2021-01-07 9:17 ` Greg KH @ 2021-01-07 9:50 ` Prashant Malani 2021-01-07 10:57 ` Greg KH 0 siblings, 1 reply; 6+ messages in thread From: Prashant Malani @ 2021-01-07 9:50 UTC (permalink / raw) To: Greg KH Cc: open list:USB NETWORKING DRIVERS, Heikki Krogerus, Benson Leung, open list Hi Greg, Thanks for taking a look at the patch. On Thu, Jan 7, 2021 at 1:16 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Wed, Jan 06, 2021 at 07:49:04PM -0800, Prashant Malani wrote: > > Generate a change uevent when the "number_of_alternate_modes" sysfs file > > for partners and plugs is updated by a port driver. > > > > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > Cc: Benson Leung <bleung@chromium.org> > > Signed-off-by: Prashant Malani <pmalani@chromium.org> > > --- > > drivers/usb/typec/class.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c > > index ebfd3113a9a8..8f77669f9cf4 100644 > > --- a/drivers/usb/typec/class.c > > +++ b/drivers/usb/typec/class.c > > @@ -766,6 +766,7 @@ int typec_partner_set_num_altmodes(struct typec_partner *partner, int num_altmod > > return ret; > > > > sysfs_notify(&partner->dev.kobj, NULL, "number_of_alternate_modes"); > > + kobject_uevent(&partner->dev.kobj, KOBJ_CHANGE); > > Shouldn't the sysfs_notify() handle the "something has changed" logic > good enough for userspace, as obviously someone is polling on the thing > (otherwise we wouldn't be calling sysfs_notify...) > > The kobject itself hasn't "changed", but rather an individual attribute > has changed. We don't want to create uevents for every individual sysfs > attribute changing values, do we? Fair point. I noticed other attributes in this source file use a similar approach (sysfs_notify + kobject_uevent) and took guidance from there in an attempt to remain consistent (though, of course, your point still stands). I'm guessing it is for processes that rely on udev events (subsystem=typec) rather than polling. > > What is preventing a normal "monitor the sysfs file" logic from working > here for anyone who wants to know that the alternate modes have changed? One limitation I can think of is that this sysfs file is hidden till it has a valid value (i.e >= 0), so a user-space process might not be able to poll on the file till it is visible (I suppose even then one could poll on the parent). Kindly disregard the patch if you reckon it is unnecessary. Best regards, -Prashant ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] usb: typec: Send uevent for num_altmodes update 2021-01-07 9:50 ` Prashant Malani @ 2021-01-07 10:57 ` Greg KH 2021-01-07 13:21 ` Greg KH 0 siblings, 1 reply; 6+ messages in thread From: Greg KH @ 2021-01-07 10:57 UTC (permalink / raw) To: Prashant Malani Cc: open list:USB NETWORKING DRIVERS, Heikki Krogerus, Benson Leung, open list On Thu, Jan 07, 2021 at 01:50:53AM -0800, Prashant Malani wrote: > Hi Greg, > > Thanks for taking a look at the patch. > > On Thu, Jan 7, 2021 at 1:16 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > On Wed, Jan 06, 2021 at 07:49:04PM -0800, Prashant Malani wrote: > > > Generate a change uevent when the "number_of_alternate_modes" sysfs file > > > for partners and plugs is updated by a port driver. > > > > > > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > > Cc: Benson Leung <bleung@chromium.org> > > > Signed-off-by: Prashant Malani <pmalani@chromium.org> > > > --- > > > drivers/usb/typec/class.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c > > > index ebfd3113a9a8..8f77669f9cf4 100644 > > > --- a/drivers/usb/typec/class.c > > > +++ b/drivers/usb/typec/class.c > > > @@ -766,6 +766,7 @@ int typec_partner_set_num_altmodes(struct typec_partner *partner, int num_altmod > > > return ret; > > > > > > sysfs_notify(&partner->dev.kobj, NULL, "number_of_alternate_modes"); > > > + kobject_uevent(&partner->dev.kobj, KOBJ_CHANGE); > > > > Shouldn't the sysfs_notify() handle the "something has changed" logic > > good enough for userspace, as obviously someone is polling on the thing > > (otherwise we wouldn't be calling sysfs_notify...) > > > > The kobject itself hasn't "changed", but rather an individual attribute > > has changed. We don't want to create uevents for every individual sysfs > > attribute changing values, do we? > > Fair point. I noticed other attributes in this source file use a > similar approach (sysfs_notify + kobject_uevent) > and took guidance from there in an attempt to remain consistent > (though, of course, your point still stands). > > I'm guessing it is for processes that rely on udev events > (subsystem=typec) rather than polling. > > > > > What is preventing a normal "monitor the sysfs file" logic from working > > here for anyone who wants to know that the alternate modes have changed? > > One limitation I can think of is that this sysfs file is hidden till > it has a valid value (i.e >= 0), so a user-space process might not > be able to poll on the file till it is visible (I suppose even then > one could poll on the parent). If the file is being added at this point in time, then yes, it is ok to send a KOBJ_CHANGE event as that is needed. Is that what is happening here? thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] usb: typec: Send uevent for num_altmodes update 2021-01-07 10:57 ` Greg KH @ 2021-01-07 13:21 ` Greg KH 2021-01-09 22:28 ` [RFC] removing explicit #define DEBUG Tom Rix 0 siblings, 1 reply; 6+ messages in thread From: Greg KH @ 2021-01-07 13:21 UTC (permalink / raw) To: Prashant Malani Cc: open list:USB NETWORKING DRIVERS, Heikki Krogerus, Benson Leung, open list On Thu, Jan 07, 2021 at 11:57:08AM +0100, Greg KH wrote: > On Thu, Jan 07, 2021 at 01:50:53AM -0800, Prashant Malani wrote: > > Hi Greg, > > > > Thanks for taking a look at the patch. > > > > On Thu, Jan 7, 2021 at 1:16 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > On Wed, Jan 06, 2021 at 07:49:04PM -0800, Prashant Malani wrote: > > > > Generate a change uevent when the "number_of_alternate_modes" sysfs file > > > > for partners and plugs is updated by a port driver. > > > > > > > > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > > > Cc: Benson Leung <bleung@chromium.org> > > > > Signed-off-by: Prashant Malani <pmalani@chromium.org> > > > > --- > > > > drivers/usb/typec/class.c | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c > > > > index ebfd3113a9a8..8f77669f9cf4 100644 > > > > --- a/drivers/usb/typec/class.c > > > > +++ b/drivers/usb/typec/class.c > > > > @@ -766,6 +766,7 @@ int typec_partner_set_num_altmodes(struct typec_partner *partner, int num_altmod > > > > return ret; > > > > > > > > sysfs_notify(&partner->dev.kobj, NULL, "number_of_alternate_modes"); > > > > + kobject_uevent(&partner->dev.kobj, KOBJ_CHANGE); > > > > > > Shouldn't the sysfs_notify() handle the "something has changed" logic > > > good enough for userspace, as obviously someone is polling on the thing > > > (otherwise we wouldn't be calling sysfs_notify...) > > > > > > The kobject itself hasn't "changed", but rather an individual attribute > > > has changed. We don't want to create uevents for every individual sysfs > > > attribute changing values, do we? > > > > Fair point. I noticed other attributes in this source file use a > > similar approach (sysfs_notify + kobject_uevent) > > and took guidance from there in an attempt to remain consistent > > (though, of course, your point still stands). > > > > I'm guessing it is for processes that rely on udev events > > (subsystem=typec) rather than polling. > > > > > > > > What is preventing a normal "monitor the sysfs file" logic from working > > > here for anyone who wants to know that the alternate modes have changed? > > > > One limitation I can think of is that this sysfs file is hidden till > > it has a valid value (i.e >= 0), so a user-space process might not > > be able to poll on the file till it is visible (I suppose even then > > one could poll on the parent). > > If the file is being added at this point in time, then yes, it is ok to > send a KOBJ_CHANGE event as that is needed. Is that what is happening > here? In looking at the code more, yes, you are right, I'll go queue this up now, thanks. greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC] removing explicit #define DEBUG 2021-01-07 13:21 ` Greg KH @ 2021-01-09 22:28 ` Tom Rix 0 siblings, 0 replies; 6+ messages in thread From: Tom Rix @ 2021-01-09 22:28 UTC (permalink / raw) To: LKML, Joe Perches In cleaning up the printf %hh's I have found a number of files that explicitly do a #define DEBUG with no comments or surrounding #ifdef CONFIG_MY_DEBUG_FEATURE/#endif If I saw this in a review, I would assume this was leftover devel code and nak the review. Finding and removing are pretty trival, i believe there are only around 10. I am bringing this up because if the existing code depends on the DEBUG, ex/ getting lucking with timing, then the change would introduce problems. Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-09 22:30 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-07 3:49 [PATCH] usb: typec: Send uevent for num_altmodes update Prashant Malani 2021-01-07 9:17 ` Greg KH 2021-01-07 9:50 ` Prashant Malani 2021-01-07 10:57 ` Greg KH 2021-01-07 13:21 ` Greg KH 2021-01-09 22:28 ` [RFC] removing explicit #define DEBUG Tom Rix
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).