RE: [PATCH 4/4] lib/hexdump.c: Allow multiple groups to be separated by lines '|'
From: Alastair D'Silva > Sent: 10 April 2019 04:17 > With the wider display format, it can become hard to identify how many > bytes into the line you are looking at. > > The patch adds new flags to hex_dump_to_buffer() and print_hex_dump() to > print vertical lines to separate every N groups of bytes. > > eg. > buf:: 454d414e 43415053|4e495f45 00584544 NAMESPAC|E_INDEX. > buf:0010: 0002| | Ugg, that is just horrid. It is enough to add an extra space if you really need the columns to be more easily counted. I'm not even sure that is needed if you are printing 32bit words. OTOH 32bit words makes 64bit values really stupid on LE systems. Bytes with extra spaces every 4 bytes is the format I prefer even for long lines. Oh, and if you are using hexdump() a lot you want a version that never uses snprintf(). David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/4] lib/hexdump.c: Allow multiple groups to be separated by lines '|'
On (04/10/19 13:17), Alastair D'Silva wrote: > With the wider display format, it can become hard to identify how many > bytes into the line you are looking at. > > The patch adds new flags to hex_dump_to_buffer() and print_hex_dump() to > print vertical lines to separate every N groups of bytes. > > eg. > buf:: 454d414e 43415053|4e495f45 00584544 NAMESPAC|E_INDEX. > buf:0010: 0002| | What if the output had |-s in it? | -ss ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 4/4] lib/hexdump.c: Allow multiple groups to be separated by lines '|'
> -Original Message- > From: David Laight > Sent: Wednesday, 10 April 2019 6:45 PM > To: 'Alastair D'Silva' ; alast...@d-silva.org > Cc: Jani Nikula ; Joonas Lahtinen > ; Rodrigo Vivi ; > David Airlie ; Daniel Vetter ; Karsten Keil > ; Jassi Brar ; Tom Lendacky > ; David S. Miller ; > Jose Abreu ; Kalle Valo > ; Stanislaw Gruszka ; > Benson Leung ; Enric Balletbo i Serra > ; James E.J. Bottomley > ; Martin K. Petersen ; > Greg Kroah-Hartman ; Alexander Viro > ; Petr Mladek ; Sergey > Senozhatsky ; Steven Rostedt > ; Andrew Morton ; > intel-...@lists.freedesktop.org; dri-de...@lists.freedesktop.org; linux- > ker...@vger.kernel.org; net...@vger.kernel.org; > ath...@lists.infradead.org; linux-wirel...@vger.kernel.org; linux- > s...@vger.kernel.org; linux-fb...@vger.kernel.org; > de...@driverdev.osuosl.org; linux-fsde...@vger.kernel.org > Subject: RE: [PATCH 4/4] lib/hexdump.c: Allow multiple groups to be > separated by lines '|' > > From: Alastair D'Silva > > Sent: 10 April 2019 04:17 > > With the wider display format, it can become hard to identify how many > > bytes into the line you are looking at. > > > > The patch adds new flags to hex_dump_to_buffer() and > print_hex_dump() > > to print vertical lines to separate every N groups of bytes. > > > > eg. > > buf:: 454d414e 43415053|4e495f45 00584544 > NAMESPAC|E_INDEX. > > buf:0010: 0002| | > > Ugg, that is just horrid. > It is enough to add an extra space if you really need the columns to be more > easily counted. > I did consider that, but it would be a more invasive change, as the buffer length required would differ based on the flags. > I'm not even sure that is needed if you are printing 32bit words. > OTOH 32bit words makes 64bit values really stupid on LE systems. > Bytes with extra spaces every 4 bytes is the format I prefer even for long > lines. > > Oh, and if you are using hexdump() a lot you want a version that never uses > snprintf(). > > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, > MK1 1PT, UK Registration No: 1397386 (Wales) > > > --- > This email has been checked for viruses by AVG. > https://www.avg.com ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: fbtft: fix alignment should match open parenthesis
Fix checkpatch warning "Alignment should match open parenthesis". Signed-off-by: Himadri Pandya --- drivers/staging/fbtft/fb_tinylcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/fbtft/fb_tinylcd.c b/drivers/staging/fbtft/fb_tinylcd.c index 9469248f2c50..60cda57bcb33 100644 --- a/drivers/staging/fbtft/fb_tinylcd.c +++ b/drivers/staging/fbtft/fb_tinylcd.c @@ -38,7 +38,7 @@ static int init_display(struct fbtft_par *par) write_reg(par, 0xE5, 0x00); write_reg(par, 0xF0, 0x36, 0xA5, 0x53); write_reg(par, 0xE0, 0x00, 0x35, 0x33, 0x00, 0x00, 0x00, - 0x00, 0x35, 0x33, 0x00, 0x00, 0x00); + 0x00, 0x35, 0x33, 0x00, 0x00, 0x00); write_reg(par, MIPI_DCS_SET_PIXEL_FORMAT, 0x55); write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE); udelay(250); -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH 22/68] vfs: Convert binderfs to use the new mount API
Dan Carpenter wrote: > > + struct ipc_namespace *ipc_ns = fc->s_fs_info; > > + put_ipc_ns(ipc_ns); > > I feel like put_ipc_ns(info->ipc_ns) would be more readable. Not so much more readable as more correct. David ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: gasket: replace symbolic permissions with octal permissions
Resolve checkpatch warning for using symbolic permissions by replacing
them with octal permissions.
Signed-off-by: Himadri Pandya
---
drivers/staging/gasket/gasket_sysfs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/gasket/gasket_sysfs.h
b/drivers/staging/gasket/gasket_sysfs.h
index 1d0eed66a7f4..28dc422d388c 100644
--- a/drivers/staging/gasket/gasket_sysfs.h
+++ b/drivers/staging/gasket/gasket_sysfs.h
@@ -75,7 +75,7 @@ struct gasket_sysfs_attribute {
#define GASKET_SYSFS_RO(_name, _show_function, _attr_type)
\
{ \
- .attr = __ATTR(_name, S_IRUGO, _show_function, NULL), \
+ .attr = __ATTR(_name, 0444, _show_function, NULL), \
.data.attr_type = _attr_type \
}
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: fbtft: fix alignment should match open parenthesis
On Wed, Apr 10, 2019 at 03:30:22PM +0530, Himadri Pandya wrote: > Fix checkpatch warning "Alignment should match open parenthesis". > > Signed-off-by: Himadri Pandya > --- > drivers/staging/fbtft/fb_tinylcd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/fbtft/fb_tinylcd.c > b/drivers/staging/fbtft/fb_tinylcd.c > index 9469248f2c50..60cda57bcb33 100644 > --- a/drivers/staging/fbtft/fb_tinylcd.c > +++ b/drivers/staging/fbtft/fb_tinylcd.c > @@ -38,7 +38,7 @@ static int init_display(struct fbtft_par *par) > write_reg(par, 0xE5, 0x00); > write_reg(par, 0xF0, 0x36, 0xA5, 0x53); > write_reg(par, 0xE0, 0x00, 0x35, 0x33, 0x00, 0x00, 0x00, > -0x00, 0x35, 0x33, 0x00, 0x00, 0x00); > + 0x00, 0x35, 0x33, 0x00, 0x00, 0x00); The original code was written that way deliberately. I don't know why it's broken up like that but it probably means something to people who have read the hardware spec. I would leave it as-is. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: media: zoran: fix trailing whitespaces
On 4/8/19 8:51 AM, Dan Carpenter wrote: > On Mon, Apr 08, 2019 at 01:35:30AM -0300, Arthur Moraes do Lago wrote: >> There were several form feeds(^L) in the file, this patch removes them >> to conform with the coding style. >> >> Signed-off-by: Arthur Moraes do Lago >> --- >> drivers/staging/media/zoran/videocodec.h | 10 +- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/staging/media/zoran/videocodec.h >> b/drivers/staging/media/zoran/videocodec.h >> index 4946791fce0d..c37c0e4fb624 100644 >> --- a/drivers/staging/media/zoran/videocodec.h >> +++ b/drivers/staging/media/zoran/videocodec.h >> @@ -49,7 +49,7 @@ >> device dependent and vary between MJPEG/MPEG/WAVELET/... devices. () >> >> >> */ >> - >> + >> > > Checkpatch also complains if you have two consecutive blank lines so > really we should just delete the lines entirely... But this driver is > scheduled for deletion in May. Let's not bother with cleaning it up. > > regards, > dan carpenter > I will in fact post a pull request to remove this driver today. Regards, Hans ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: fbtft: fix alignment should match open parenthesis
On 10/04/19 4:12 PM, Dan Carpenter wrote: On Wed, Apr 10, 2019 at 03:30:22PM +0530, Himadri Pandya wrote: Fix checkpatch warning "Alignment should match open parenthesis". Signed-off-by: Himadri Pandya --- drivers/staging/fbtft/fb_tinylcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/fbtft/fb_tinylcd.c b/drivers/staging/fbtft/fb_tinylcd.c index 9469248f2c50..60cda57bcb33 100644 --- a/drivers/staging/fbtft/fb_tinylcd.c +++ b/drivers/staging/fbtft/fb_tinylcd.c @@ -38,7 +38,7 @@ static int init_display(struct fbtft_par *par) write_reg(par, 0xE5, 0x00); write_reg(par, 0xF0, 0x36, 0xA5, 0x53); write_reg(par, 0xE0, 0x00, 0x35, 0x33, 0x00, 0x00, 0x00, - 0x00, 0x35, 0x33, 0x00, 0x00, 0x00); + 0x00, 0x35, 0x33, 0x00, 0x00, 0x00); The original code was written that way deliberately. I don't know why it's broken up like that but it probably means something to people who have read the hardware spec. I would leave it as-is. regards, dan carpenter Okay. Thank you for reviewing it. - Himadri ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vc04_services: add missing __user annotations
On 5/04/19 3:12 PM, Stefan Wahren wrote: Hi Jasminko, Am 24.03.19 um 18:17 schrieb Jasminko Dedic: This patch fixes the following sparse warnings by adding missing __user annotations. It also cleans up two related unnecessary casts by reuseing casts already made a few lines up. Remaining sparse warnings are of a different type. vchiq_arm.c:1606:14: warning: incorrect type in assignment (different address spaces) vchiq_arm.c:1606:14:expected struct vchiq_queue_message *args vchiq_arm.c:1606:14:got void [noderef] * vchiq_arm.c:1612:13: warning: incorrect type in argument 1 (different address spaces) vchiq_arm.c:1612:13:expected void const volatile [noderef] * vchiq_arm.c:1612:13:got unsigned int * vchiq_arm.c:1613:13: warning: incorrect type in argument 1 (different address spaces) vchiq_arm.c:1613:13:expected void const volatile [noderef] * vchiq_arm.c:1613:13:got unsigned int * vchiq_arm.c:1614:13: warning: incorrect type in argument 1 (different address spaces) vchiq_arm.c:1614:13:expected void const volatile [noderef] * vchiq_arm.c:1614:13:got struct vchiq_element const [noderef] ** vchiq_arm.c:1638:21: warning: incorrect type in argument 1 (different address spaces) vchiq_arm.c:1638:21:expected void const volatile [noderef] * vchiq_arm.c:1638:21:got struct vchiq_element const [noderef] ** Signed-off-by: Jasminko Dedic maybe this sounds overcautious to you, but did you test your patch on a Raspberry Pi? Hello, I've tested it on a Raspberry Pi 3 b, with a 32 bit linux-next kernel, and with an aarch64 rpi-5.0.y kernel, without any issues. I was not able to boot any arch64 linux-next kernel on the Raspberry Pi, and I'm not sure why. I tried mainline u-boot and without u-boot. If anyone has any tips for how to do that, I can test the patch with an aarch64 linux-next kernel too. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vc04_services: add missing __user annotations
Hi Jas, Am 10.04.19 um 15:45 schrieb Jas: > On 5/04/19 3:12 PM, Stefan Wahren wrote: >> Hi Jasminko, >> >> maybe this sounds overcautious to you, but did you test your patch on a >> Raspberry Pi? >> > > Hello, > > I've tested it on a Raspberry Pi 3 b, with a 32 bit linux-next kernel, > and with an aarch64 rpi-5.0.y kernel, without any issues. great > > I was not able to boot any arch64 linux-next kernel on the Raspberry > Pi, and I'm not sure why. I tried mainline u-boot and without u-boot. > If anyone has any tips for how to do that, I can test the patch with > an aarch64 linux-next kernel too. Hm, kernelci.org looks okay so there shouldn't be a regression. Maybe this [1] is helpful, but it doesn't explain everything. Nevertheless, i'm fine with this change: Acked-by: Stefan Wahren Thanks Stefan [1] - https://gist.github.com/lategoodbye/c7317a42bf7f9c07f5a91baed8c68f75 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC] staging: wilc1000: give usleep_range a range
Hi Nicolas On 4/8/19 6:36 PM, Nicholas Mc Guire wrote: > On Mon, Apr 08, 2019 at 09:10:00PM +, adham.aboza...@microchip.com wrote: >> Hi Nicholas >> >> On 4/6/19 5:01 AM, Nicholas Mc Guire wrote: >>> External E-Mail >>> >>> >>> Someone that knows the motivation for setting the time to 2 millisecond >>> might need to check if the 2 milliseconds where seen as tollerable max or >>> min - I'm assuming it was the min so extending. >> 2 msec is the time the chip takes to wake up from sleep. >> >> Increasing the maximum to 5 msec will impact the throughput since this call >> is on the transmit path. >> > ok - would it be tollerable to make it 2 - 2.5 ms ? > even that would allow for the hrtimer subsystem to optimize > a lot. In any case the min==max case gives you very little > if you run a test-case with usleep_range(1000,1000) and > a loop with usleep_range(1000,2000) and look at the distribution > you will have a hard time seeing any difference. yes, I believe 2.5 shouldn't be a problem. Thanks, Adham ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: power_supply: Use struct_size() helper
Hi Johan, On 4/4/19 1:57 AM, Johan Hovold wrote: > > This patch looks good, but I noticed a bug here in the current code, > which should be fixed before applying this clean up. > > sizeof(req) should have been sizeof(*req) above. > Good catch. >> - sizeof(struct gb_power_supply_props_desc), >> + sizeof(req), >> + struct_size(resp, props, props_count), >> GFP_KERNEL); >> if (!op) >> return -ENOMEM; > > I've just submitted a fix (and CCed you as well). > > Would you mind respinning on top of that one? > Yep. I'll send v2 shortly. Thanks -- Gustavo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: greybus: power_supply: Use struct_size() helper
Johan, On 4/4/19 2:24 AM, Johan Hovold wrote: > On Thu, Apr 04, 2019 at 08:09:51AM +0100, Rui Miguel Silva wrote: >> Hi Gustavo, >> Thanks a lot for the patch. >> >> On Wed 03 Apr 2019 at 21:58, Gustavo A. R. Silva wrote: >>> Make use of the struct_size() helper instead of an open-coded >>> version >>> in order to avoid any potential type mistakes, in particular in >>> the >>> context in which this code is being used. >>> >>> So, replace code of the following form: >>> >>> sizeof(*resp) + props_count * sizeof(struct >>> gb_power_supply_props_desc) >>> >>> with: >>> >>> struct_size(resp, props, props_count) >>> >>> This code was detected with the help of Coccinelle. >>> >>> Signed-off-by: Gustavo A. R. Silva >> >> What are the odds of 2 people changing same code in greybus in >> the same day :). > > Well, I only noticed the bug in the current code, when reviewing > Gustavo's diff. ;) > Apparently, your patch hasn't been applied to any tree yet. So, I'll wait for it to be applied before sending v2. Thanks -- Gustavo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: rtl8723bs: Remove typedef in struct sdio_data
On Wed, Apr 10, 2019 at 09:49:54AM +0300, Dan Carpenter wrote: > On Tue, Apr 09, 2019 at 11:16:17AM -0500, Madhumitha Prabakaran wrote: > > diff --git a/drivers/staging/rtl8723bs/include/drv_types.h > > b/drivers/staging/rtl8723bs/include/drv_types.h > > index bafb2c30e7fb..b0623c936940 100644 > > --- a/drivers/staging/rtl8723bs/include/drv_types.h > > +++ b/drivers/staging/rtl8723bs/include/drv_types.h > > @@ -220,7 +220,7 @@ struct registry_priv > > #define BSSID_SZ(field) sizeof(((struct wlan_bssid_ex *) 0)->field) > > > > #include > > -#define INTF_DATA SDIO_DATA > > +#define INTF_DATA struct sdio_data > > > > Just get rid of INTF_DATA data as well. It's only used once a bit lower > in the file. (Get rid of the ifdef around it). But, reference of INTF_DATA is also included in file ./drivers/staging/rtl8723bs/include/drv_types.h:487: return &dvobj->intf_data.func->dev; thanks for the review madhumitha > > regards, > dan carpenter > > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RFC] staging: wilc1000: give usleep_range a range
On Wed, Apr 10, 2019 at 06:31:21PM +, adham.aboza...@microchip.com wrote: > Hi Nicolas > > On 4/8/19 6:36 PM, Nicholas Mc Guire wrote: > > On Mon, Apr 08, 2019 at 09:10:00PM +, adham.aboza...@microchip.com > > wrote: > >> Hi Nicholas > >> > >> On 4/6/19 5:01 AM, Nicholas Mc Guire wrote: > >>> External E-Mail > >>> > >>> > >>> Someone that knows the motivation for setting the time to 2 millisecond > >>> might need to check if the 2 milliseconds where seen as tollerable max or > >>> min - I'm assuming it was the min so extending. > >> 2 msec is the time the chip takes to wake up from sleep. > >> > >> Increasing the maximum to 5 msec will impact the throughput since this > >> call is on the transmit path. > >> > > ok - would it be tollerable to make it 2 - 2.5 ms ? > > even that would allow for the hrtimer subsystem to optimize > > a lot. In any case the min==max case gives you very little > > if you run a test-case with usleep_range(1000,1000) and > > a loop with usleep_range(1000,2000) and look at the distribution > > you will have a hard time seeing any difference. > > yes, I believe 2.5 shouldn't be a problem. > thanks - will send out a V2 then shortly. thx! hofrat ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V2] staging: wilc1000: give usleep_range a range
From: Nicholas Mc Guire
usleep_range() is called in non-atomic context so there is little point
in setting min==max as the jitter of hrtimer is determined by interruptions
anyway. usleep_range can only perform the intended coalescence if some
room for placing the hrtimer is provided. Given the range of milliseconds
the delay will be 2+ anyway - so make it 2-2.5 ms which gives hrtimers
space to optimize without negatively impacting performance.
Signed-off-by: Nicholas Mc Guire
---
Problem located with an experimental coccinelle script
./drivers/staging/wilc1000/wilc_wlan.c:411:4-16: WARNING: inefficient
usleep_range with range 0 (min==max)
./drivers/staging/wilc1000/wilc_wlan.c:426:4-16: WARNING: inefficient
usleep_range with range 0 (min==max)
V2: Based on feedback from Adham Abozaeid that
since this is in the transmit path 5ms could impact throughput but adding
500 microseconds should be tolerable.
Patch was compile tested with: x86_64_defconfig + Staging=y,
WILC1000_SDIO=m, WILC1000_SPI=m, WILC1000=m
Patch is against 5.1-rc4 (localversion-next is -next-20190410)
drivers/staging/wilc1000/wilc_wlan.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/wilc1000/wilc_wlan.c
b/drivers/staging/wilc1000/wilc_wlan.c
index c238969..42da533 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -408,7 +408,7 @@ void chip_wakeup(struct wilc *wilc)
wilc->hif_func->hif_write_reg(wilc, 1, reg & ~BIT(1));
do {
- usleep_range(2 * 1000, 2 * 1000);
+ usleep_range(2000, 2500);
wilc_get_chipid(wilc, true);
} while (wilc_get_chipid(wilc, true) == 0);
} while (wilc_get_chipid(wilc, true) == 0);
@@ -423,7 +423,7 @@ void chip_wakeup(struct wilc *wilc)
&clk_status_reg);
while ((clk_status_reg & 0x1) == 0) {
- usleep_range(2 * 1000, 2 * 1000);
+ usleep_range(2000, 2500);
wilc->hif_func->hif_read_reg(wilc, 0xf1,
&clk_status_reg);
--
2.1.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
