Hello Nicholas, thank you for your contribution, I really appreciate it !
See inline comments below.
On Sat, Apr 27, 2019 at 12:32 AM Nicholas Mc Guire wrote:
>
> wait_for_completion_timeout() returns unsigned long (0 on timeout or
> remaining jiffies) not int.
Nice catch !
> thus there is no n
On Sat, Apr 27, 2019 at 3:01 AM Nicholas Mc Guire wrote:
> > > (some unrelated sparse warnings (cast to restricted __be16))
> >
> > That sounds interesting too. Could you provide more details?
>
> make C=1
> drivers/staging/fieldbus/anybuss/host.c:1350:25: warning: cast to restricted
> __be16
> d
On Sat, Apr 27, 2019 at 7:18 AM Nicholas Mc Guire wrote:
>
> so the issue is simply that the endiannes anotatoin is missing even
> though the conversion is being done - with other words there is no code
> lvel funcitonal bug here but rather sparse needs the anotation to verify
> correctness and th
d by a short
paragraph outlining what the patch does to fix it.
>
> Signed-off-by: Nicholas Mc Guire
> ---
>
> Problem located with experimental API conformance checking cocci script
>
> V2: The original patch's logic was wrong as it was skipping the
> fall-throug
Thanks for the contibution! See inline.
On Sat, Apr 27, 2019 at 10:39 PM Nicholas Mc Guire wrote:
>
> While the endiannes is being handled correctly sparse was unhappy with
> the missing annotation as be16_to_cpu() expects a __be16.
Your commit message has room for improvement here. See my remar
On Mon, Apr 29, 2019 at 2:11 AM Nicholas Mc Guire wrote:
>
> wait_for_completion_timeout() returns unsigned long (0 on timeout or
> remaining jiffies) not int - so this type error allows for a
> theoretically int overflow - though not in this case where TIMEOUT is
> only HZ*2). To fix this type in
On Mon, Apr 29, 2019 at 2:11 AM Nicholas Mc Guire wrote:
>
> V2: As requested by Sven Van Asbroeck make the
> impact of the patch clear in the commit message.
Thank you, but did you miss my comment about creating a local variable
instead? See:
https://lkml.org/lkml/201
On Tue, Apr 30, 2019 at 12:19 AM Al Viro wrote:
>
> ... not that there's much sense keeping ->fieldbus_type in host-endian,
> while we are at it.
Interesting! Suppose we make device->fieldbus_type bus-endian.
Then the endinan-ness conversion either needs to happen in
bus_match() (and we'd have to
On Tue, Apr 30, 2019 at 10:03 AM Greg Kroah-Hartman
wrote:
>
> Keep it bus-endian, as that's the "normal" way bus structures work (like
> PCI and USB for example), and that should be in a documented, and
> consistent, form, right?
>
> Then do the conversion when you access the field from within th
On Tue, Apr 30, 2019 at 10:22 AM Sven Van Asbroeck wrote:
>
> Ah ok, it's like a standard way of implementing a bus. Sounds good, I'll
> spin a patch to conform to it. And while I'm at it, I'll rename fieldbus_type
> because it can be confused with another fieldbu
Hi Oscar, thank you for your contribution! I have a question, see inline.
On Fri, May 17, 2019 at 1:50 PM Oscar Gomez Fuente
wrote:
>
> These changes solve a warning realated to an incorrect type inilizer in the
> function
> fieldbus_poll.
>
Where is this warning generated? Could you provide so
On Mon, May 20, 2019 at 2:35 PM Sven Van Asbroeck wrote:
>
> Hi Oscar, thank you for your contribution! I have a question, see inline.
>
> On Fri, May 17, 2019 at 1:50 PM Oscar Gomez Fuente
> wrote:
> >
> > These changes solve a warning realated to an incorrect type in
On Fri, May 17, 2019 at 1:50 PM Oscar Gomez Fuente
wrote:
>
> These changes solve a warning realated to an incorrect type inilizer in the
> function
> fieldbus_poll.
>
> Signed-off-by: Oscar Gomez Fuente
> ---
I've reviewed your patch and tested it on a live system. Everything looks good.
Howev
From: Oscar Gomez Fuente
->poll() functions should return __poll_t, but the fieldbus
core's poll() does not. This generates a sparse warning.
Fix the ->poll() return value, and use recommended __poll_t
constants (EPOLLxxx).
Signed-off-by: Oscar Gomez Fuente
---
drivers/staging/fieldbus/dev_co
On Tue, May 21, 2019 at 10:17 AM Greg KH wrote:
>
>
> Greg already took this patch a while ago :)
>
Thanks for bringing that up Greg, I'll double-check your tree next time.
Oscar, please ignore the v3 patch I posted.
Sven
___
devel mailing list
de...@
On Tue, May 21, 2019 at 10:20 AM Sven Van Asbroeck wrote:
>
> From: Oscar Gomez Fuente
>
> ->poll() functions should return __poll_t, but the fieldbus
> core's poll() does not. This generates a sparse warning.
>
> Fix the ->poll() return value, and use recommende
On Tue, May 21, 2019 at 10:37 AM Dan Carpenter wrote:
>
>
> If you're resending someone's patch, you have to add your own Signed off
> by line as well. Everyone who touches a patch has to sign that they
> didn't add any of SCO's all powerful UnixWare source code into the
> patch.
>
Thank you Dan
The regmap's context (stored as 'void *') consists of a pointer to
__iomem. Mixing __iomem and non-__iomem addresses generates
sparse warnings.
Fix by using __force when converting to/from 'void __iomem *' and
the regmap's context.
Signed-off-by: Sven Van Asbroeck
On Tue, May 21, 2019 at 11:13 AM Dan Carpenter wrote:
>
> There is no need to use __force. Just:
>
> void __iomem *base = (void __iomem *)context;
>
> For the rest as well.
Yes, that appears to work for 'void *' -> __iomem, but not the other way around:
+ return devm_regmap_init(d
On Tue, May 21, 2019 at 11:42 AM Greg KH wrote:
>
> Ick, if you are using __force, almost always something is wrong.
>
What if I create a separate structure for the regmap context ?
struct anybus_regmap_context {
void __iomem *base;
};
Then just store the base pointer inside the struct,
On Tue, May 21, 2019 at 12:24 PM Greg KH wrote:
>
> what is so odd about this code that makes you have to jump through
> strange hoops that no other driver has to?
>
Basically because it creates a regmap which accesses __iomem memory,
instead of i2c/spi.
This was done because future hardware in
On Tue, May 21, 2019 at 12:53 PM Sven Van Asbroeck wrote:
>
> On Tue, May 21, 2019 at 12:24 PM Greg KH wrote:
> >
> > what is so odd about this code that makes you have to jump through
> > strange hoops that no other driver has to?
> >
>
> Basically becaus
place the custom regmap with the existing kernel abstraction.
As a pleasant side-effect, sparse warnings now disappear.
Signed-off-by: Sven Van Asbroeck
---
.../staging/fieldbus/anybuss/arcx-anybus.c| 44 ++-
1 file changed, 13 insertions(+), 31 deletions(-)
diff --git a/dr
s address in an
> array of pointers to const struct attribute_group, and the only usage of
> can_power_ops is to assign its address to the 'ops' field in the
> regulator_desc struct, which is a pointer to const struct regulator_ops.
>
> Signed-off-by: Rikard Fa
s/arcx-anybus.c:248:1-14: WARNING: Use
> devm_platform_ioremap_resource for cd -> cpld_base
>
> Signed-off-by: Hariprasad Kelam
> ---
> drivers/staging/fieldbus/anybuss/arcx-anybus.c | 8 ++--
> 1 file changed, 2 insertions(+), 6 deletions(-)
Reviewed-by: Sven
On Wed, Oct 16, 2019 at 4:50 AM YueHaibing wrote:
>
> Use devm_platform_ioremap_resource() to simplify the code a bit.
> This is detected by coccinelle.
>
> Signed-off-by: YueHaibing
> ---
> drivers/staging/fieldbus/anybuss/arcx-anybus.c | 8 ++--
> 1 file changed, 2 insertions(+), 6 deletio
On Sat, Oct 26, 2019 at 7:52 PM Cristiane Naves
wrote:
>
> Use devm_platform_ioremap_resource helper which wraps
> platform_get_resource() and devm_ioremap_resource() together. Issue
> found by coccicheck.
>
> Signed-off-by: Cristiane Naves
This is a duplicate of:
https://www.spinics.net/lists/l
Hi Greg, friendly reminder... Did you miss the patch review below, or
is there a reason
why this isn't getting queued?
There seems to be a crowd chasing down this type of warnings, resulting
in multiple duplicates.
On Tue, Oct 8, 2019 at 9:31 AM Sven Van Asbroeck wrote:
>
> On Tue,
On Tue, Oct 29, 2019 at 4:03 AM Greg Kroah-Hartman
wrote:
>
> This has been in my tree already for a while, can you verify it is all
> ok?
>
All good. I see you took Cristiane's version, not hariprasad's or YueHaibing's.
As long as one version lands, all is well :)
___
us/anybuss/arcx-anybus.c | 8 ++--
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
Reviewed-by: Sven Van Asbroeck
Tested-by: Sven Van Asbroeck
Greg has already queued this patch, but the link embedded in
his commit message should point people here.
___
On Thu, May 23, 2019 at 5:09 AM Dan Carpenter wrote:
>
> Sven, you should add yourself to the MAINTAINERS file.
Greg, what do you think?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverd
Add myself as the maintainer of the fieldbus subsystem.
Signed-off-by: Sven Van Asbroeck
---
MAINTAINERS | 5 +
1 file changed, 5 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 5cfbea4ce575..1cac53bced08 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14905,6 +14905,11 @@ L
Add myself as the maintainer of the anybuss bus driver, and its client
drivers.
Signed-off-by: Sven Van Asbroeck
---
MAINTAINERS | 5 +
1 file changed, 5 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 1cac53bced08..68d49623186f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
On Thu, May 23, 2019 at 4:00 PM Joe Perches wrote:
> patch 2/2 specifically covers the anybuss directory,
> but the Documentation directory has no matching pattern.
Thank you for spotting that, I will re-spin the set.
>
> trivia: anybuss looks like a misspelling.
> It might be better as anybus-s
Add myself as the maintainer of the fieldbus subsystem.
Signed-off-by: Sven Van Asbroeck
---
MAINTAINERS | 6 ++
1 file changed, 6 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 5cfbea4ce575..50e164041e94 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14905,6 +14905,12 @@ L
Add myself as the maintainer of the anybuss bus driver, and its client
drivers.
Signed-off-by: Sven Van Asbroeck
---
MAINTAINERS | 5 +
1 file changed, 5 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 50e164041e94..2b9223be10b6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
Add myself as the maintainer of the anybuss bus driver, and its client
drivers.
Signed-off-by: Sven Van Asbroeck
---
MAINTAINERS | 5 +
1 file changed, 5 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 50e164041e94..2b9223be10b6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
Add myself as the maintainer of the fieldbus subsystem.
Signed-off-by: Sven Van Asbroeck
---
v3:
resend, no changes, forgot to attach history to patch set
v2:
add Documentation directory to maintainer entry for fieldbus,
suggested by Joe Perches
v1
On Sat, May 25, 2019 at 12:20 AM Mao Wenan wrote:
>
> The variable 'status' is not used any more, remve it.
> /* do the transfers for this message */
> list_for_each_entry(transfer, &m->transfers, transfer_list) {
> if (transfer->tx_buf == NULL && transfer->rx_buf == NULL &&
>
On Fri, May 24, 2019 at 2:04 AM Nishka Dasgupta
wrote:
>
> The return variable is used only twice (in two different branches), and
> both times it is assigned the same constant value. These can therefore
> be merged into the same assignment, placed at the point that both
> these branches (and no o
Hello YueHaibing,
On Tue, May 28, 2019 at 9:33 AM YueHaibing wrote:
>
> Fix gcc build error while CONFIG_REGMAP_MMIO is not set
>
> drivers/staging/fieldbus/anybuss/arcx-anybus.o: In function
> `controller_probe':
> arcx-anybus.c:(.text+0x9d6): undefined reference to
> `__devm_regmap_init_mmio_
On Tue, May 28, 2019 at 10:31 AM YueHaibing wrote:
>
> Fix gcc build error while CONFIG_REGMAP_MMIO is not set
>
checkpatch.pl errors remain:
$ ./scripts/checkpatch.pl < ~/Downloads/YueHaibing.eml
ERROR: DOS line endings
#92: FILE: drivers/staging/fieldbus/anybuss/Kconfig:17:
+^Iselect REGMAP_MM
y, dos2unix fixes it up just fine.
For the v2 patch:
Reviewed-by: Sven Van Asbroeck
Thank you YueHaibing.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Sven Van Asbroeck wrote:
> For the v2 patch:
> Reviewed-by: Sven Van Asbroeck
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Hi Andy, thank you for the patch ! See below.
On Thu, Jul 30, 2020 at 11:27 AM Andy Shevchenko
wrote:
>
> -struct msg_mac_addr {
> - u8 addr[6];
> -};
I would prefer to keep this structure. It's conceptually important,
because it describes the binary layout of a message going to a
peripher
Hi Jing, thank you for your patch.
Reviewed-by: Sven Van Asbroeck
On Mon, Oct 12, 2020 at 9:17 AM Jing Xiangfeng wrote:
>
> In current code, controller_probe() misses to call ida_simple_remove()
> in an error path. Jump to correct label to fix it.
>
> Fixes: 17614978ed34 (&qu
Hi Andy, thank you for the patch !
On Tue, Oct 27, 2020 at 2:34 PM Andy Shevchenko
wrote:
>
> + return snprintf(buf, max_size, "%pM\n", response.addr);
Judging from a few Outreachy patches that have hit my inbox, snprintf() is
considered unsafe in a sysfs_get callback. It should be replace
47 matches
Mail list logo
