On Fri, May 14, 2021 at 08:42:16PM +0530, Shreyansh Chouhan wrote:
> On Fri, May 14, 2021 at 04:30:23PM +0200, Greg KH wrote:
> > On Fri, May 14, 2021 at 07:53:57PM +0530, Shreyansh Chouhan wrote:
> > > On Fri, May 14, 2021 at 04:05:32PM +0200, Greg KH wrote:
> > > > On Fri, May 14, 2021 at 07:18:38PM +0530, Shreyansh Chouhan wrote:
> > > > > On Fri, May 14, 2021 at 03:36:25PM +0200, Greg KH wrote:
> > > > > > On Fri, May 14, 2021 at 07:00:39PM +0530, Shreyansh Chouhan wrote:
> > > > > > > The gb_loopback_stats_attrs macro, (defined in loopback.c,) is a
> > > > > > > multiline macro whose statements were not enclosed in a do while
> > > > > > > loop.
> > > > > > >
> > > > > > > This patch adds a do while loop around the statements of the said
> > > > > > > macro.
> > > > > > >
> > > > > > > Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh630(a)gmail.com>
> > > > > > > ---
> > > > > > > drivers/staging/greybus/loopback.c | 10 ++++++----
> > > > > > > 1 file changed, 6 insertions(+), 4 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
> > > > > > > index 2471448ba42a..c88ef3e894fa 100644
> > > > > > > --- a/drivers/staging/greybus/loopback.c
> > > > > > > +++ b/drivers/staging/greybus/loopback.c
> > > > > > > @@ -162,10 +162,12 @@ static ssize_t name##_avg_show(struct device *dev, \
> > > > > > > } \
> > > > > > > static DEVICE_ATTR_RO(name##_avg)
> > > > > > >
> > > > > > > -#define gb_loopback_stats_attrs(field) \
> > > > > > > - gb_loopback_ro_stats_attr(field, min, u); \
> > > > > > > - gb_loopback_ro_stats_attr(field, max, u); \
> > > > > > > - gb_loopback_ro_avg_attr(field)
> > > > > > > +#define gb_loopback_stats_attrs(field) \
> > > > > > > + do { \
> > > > > > > + gb_loopback_ro_stats_attr(field, min, u); \
> > > > > > > + gb_loopback_ro_stats_attr(field, max, u); \
> > > > > > > + gb_loopback_ro_avg_attr(field); \
> > > > > > > + } while (0)
> > > > > > >
> > > > > > > #define gb_loopback_attr(field, type) \
> > > > > > > static ssize_t field##_show(struct device *dev, \
> > > > > > > --
> > > > > > > 2.31.1
> > > > > > >
> > > > > > >
> > > > > >
> > > > > > Did you test build this change?
> > > > >
> > > > > I built the module using make -C . M=drivers/staging/greybus to test
> > > > > build it. I didn't get any errors.
> > > >
> > > > Really? Can you provide the full build output for this file with your
> > > > change? I don't think you really built this file for the obvious
> > > > reasons...
> > >
> > > I ran make -C . M=drivers/staging/greybus
> > >
> > > I got a three line output saying:
> > > make: Entering directory '/work/linux'
> > > MODPOST drivers/staging/greybus//Module.symvers
> > > make: Leaving directory '/work/linux'
> > >
> > > I just tried rebuilding the kernel with CONFIG_GREYBUS=m, and now I can
> > > see what you are talking about. Why weren't these errors reported when I
> > > ran the previous make command? Does that too check for the config
> > > variables even when I specifically asked it to build a module?
> >
> > You were just asking it to build a subdirectory, not a specific
> > individual file, and when you do that it looks at the configuration
> > settings.
> >
>
> I see.
>
> > It's always good to ensure that you actually build the files you modify
> > before sending patches out.
>
> Sorry, I googled about building a single module, and thought running
> that command would have built it. Moreover, since the change was so
> simple I didn't suspect anything when it got built correctly the first
> time around.
>
> I didn't look at how/where was the macro called and missed a very
> obvious error. Now that I have looked at it, the only way I can think of
> fixing this is changing the macro to a (inline?) function. Will
> that be a desirable change?
No, it can't be a function, the code is fine as-is, checkpatch is just a
perl script and does not always know what needs to be done.
thanks,
greg k-h
On Fri, May 14, 2021 at 07:53:57PM +0530, Shreyansh Chouhan wrote:
> On Fri, May 14, 2021 at 04:05:32PM +0200, Greg KH wrote:
> > On Fri, May 14, 2021 at 07:18:38PM +0530, Shreyansh Chouhan wrote:
> > > On Fri, May 14, 2021 at 03:36:25PM +0200, Greg KH wrote:
> > > > On Fri, May 14, 2021 at 07:00:39PM +0530, Shreyansh Chouhan wrote:
> > > > > The gb_loopback_stats_attrs macro, (defined in loopback.c,) is a
> > > > > multiline macro whose statements were not enclosed in a do while
> > > > > loop.
> > > > >
> > > > > This patch adds a do while loop around the statements of the said
> > > > > macro.
> > > > >
> > > > > Signed-off-by: Shreyansh Chouhan <chouhan.shreyansh630(a)gmail.com>
> > > > > ---
> > > > > drivers/staging/greybus/loopback.c | 10 ++++++----
> > > > > 1 file changed, 6 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
> > > > > index 2471448ba42a..c88ef3e894fa 100644
> > > > > --- a/drivers/staging/greybus/loopback.c
> > > > > +++ b/drivers/staging/greybus/loopback.c
> > > > > @@ -162,10 +162,12 @@ static ssize_t name##_avg_show(struct device *dev, \
> > > > > } \
> > > > > static DEVICE_ATTR_RO(name##_avg)
> > > > >
> > > > > -#define gb_loopback_stats_attrs(field) \
> > > > > - gb_loopback_ro_stats_attr(field, min, u); \
> > > > > - gb_loopback_ro_stats_attr(field, max, u); \
> > > > > - gb_loopback_ro_avg_attr(field)
> > > > > +#define gb_loopback_stats_attrs(field) \
> > > > > + do { \
> > > > > + gb_loopback_ro_stats_attr(field, min, u); \
> > > > > + gb_loopback_ro_stats_attr(field, max, u); \
> > > > > + gb_loopback_ro_avg_attr(field); \
> > > > > + } while (0)
> > > > >
> > > > > #define gb_loopback_attr(field, type) \
> > > > > static ssize_t field##_show(struct device *dev, \
> > > > > --
> > > > > 2.31.1
> > > > >
> > > > >
> > > >
> > > > Did you test build this change?
> > >
> > > I built the module using make -C . M=drivers/staging/greybus to test
> > > build it. I didn't get any errors.
> >
> > Really? Can you provide the full build output for this file with your
> > change? I don't think you really built this file for the obvious
> > reasons...
>
> I ran make -C . M=drivers/staging/greybus
>
> I got a three line output saying:
> make: Entering directory '/work/linux'
> MODPOST drivers/staging/greybus//Module.symvers
> make: Leaving directory '/work/linux'
>
> I just tried rebuilding the kernel with CONFIG_GREYBUS=m, and now I can
> see what you are talking about. Why weren't these errors reported when I
> ran the previous make command? Does that too check for the config
> variables even when I specifically asked it to build a module?
You were just asking it to build a subdirectory, not a specific
individual file, and when you do that it looks at the configuration
settings.
It's always good to ensure that you actually build the files you modify
before sending patches out.
thanks,
greg k-h
Hey all, new member here!
As the subject line suggests, our development hardware does not support
UniPro so we were looking at a guest kernel solution with TCP/IP transport
over gbridge.
However, one of the SDIO bus devices is a wifi module that requires DMA. Is
this possible over the current gb-netlink/gbridge to your knowledge?
Cheers,
--
*—*
*Kyle Harding*
Embedded Linux Engineer, balena.io
Fix these kernel-doc complaints:
../drivers/greybus/es2.c:79: warning: bad line:
../drivers/greybus/es2.c:100: warning: cannot understand function prototype: 'struct es2_ap_dev '
es2.c:126: warning: Function parameter or member 'cdsi1_in_use' not described in 'es2_ap_dev'
Signed-off-by: Randy Dunlap <rdunlap(a)infradead.org>
Cc: Johan Hovold <johan(a)kernel.org>
Cc: Alex Elder <elder(a)kernel.org>
Cc: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
Cc: greybus-dev(a)lists.linaro.org (moderated for non-subscribers)
---
drivers/greybus/es2.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
--- linux-next-20210414.orig/drivers/greybus/es2.c
+++ linux-next-20210414/drivers/greybus/es2.c
@@ -72,11 +72,11 @@ struct es2_cport_in {
};
/**
- * es2_ap_dev - ES2 USB Bridge to AP structure
+ * struct es2_ap_dev - ES2 USB Bridge to AP structure
* @usb_dev: pointer to the USB device we are.
* @usb_intf: pointer to the USB interface we are bound to.
* @hd: pointer to our gb_host_device structure
-
+ *
* @cport_in: endpoint, urbs and buffer for cport in messages
* @cport_out_endpoint: endpoint for for cport out messages
* @cport_out_urb: array of urbs for the CPort out messages
@@ -85,7 +85,7 @@ struct es2_cport_in {
* @cport_out_urb_cancelled: array of flags indicating whether the
* corresponding @cport_out_urb is being cancelled
* @cport_out_urb_lock: locks the @cport_out_urb_busy "list"
- *
+ * @cdsi1_in_use: true if cport CDSI1 is in use
* @apb_log_task: task pointer for logging thread
* @apb_log_dentry: file system entry for the log file interface
* @apb_log_enable_dentry: file system entry for enabling logging
disable_irq() after request_irq() still has a time gap in which
interrupts can come. request_irq() with IRQF_NO_AUTOEN flag will
disable IRQ auto-enable because of requesting.
this patch is made base on "add IRQF_NO_AUTOEN for request_irq" which
is being merged: https://lore.kernel.org/patchwork/patch/1388765/
Signed-off-by: Tian Tao <tiantao6(a)hisilicon.com>
---
drivers/staging/greybus/arche-platform.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c
index e374dfc..be27ace 100644
--- a/drivers/staging/greybus/arche-platform.c
+++ b/drivers/staging/greybus/arche-platform.c
@@ -500,13 +500,13 @@ static int arche_platform_probe(struct platform_device *pdev)
arche_platform_wd_irq,
arche_platform_wd_irq_thread,
IRQF_TRIGGER_FALLING |
- IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ IRQF_TRIGGER_RISING | IRQF_ONESHOT |
+ IRQF_NO_AUTOEN,
dev_name(dev), arche_pdata);
if (ret) {
dev_err(dev, "failed to request wake detect IRQ %d\n", ret);
return ret;
}
- disable_irq(arche_pdata->wake_detect_irq);
ret = device_create_file(dev, &dev_attr_state);
if (ret) {
--
2.7.4