[driver-core:debugfs_cleanup 24/55] drivers/gpu//drm/msm/disp/dpu1/dpu_kms.c:227:3: warning: 'return' with no value, in function returning non-void
tree: https://kernel.googlesource.com/pub/scm/linux/kernel/git/gregkh/driver-core.git debugfs_cleanup head: a0738df2bf14f1a7151465012ab48e55c317a4f6 commit: 2b24e4775e3c329bb9fdc4539a50178139d26e40 [24/55] drm: make .debugfs_init and drm_debugfs_create_files() return void config: arm-allmodconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (GCC) 7.4.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 2b24e4775e3c329bb9fdc4539a50178139d26e40 # save the attached .config to linux build tree GCC_VERSION=7.4.0 make.cross ARCH=arm If you fix the issue, kindly add following tag Reported-by: kbuild test robot All warnings (new ones prefixed by >>): drivers/gpu//drm/msm/disp/dpu1/dpu_kms.c: In function 'dpu_kms_debugfs_init': >> drivers/gpu//drm/msm/disp/dpu1/dpu_kms.c:227:3: warning: 'return' with no >> value, in function returning non-void [-Wreturn-type] return; ^~ drivers/gpu//drm/msm/disp/dpu1/dpu_kms.c:220:12: note: declared here static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor) ^~~~ drivers/gpu//drm/msm/disp/dpu1/dpu_kms.c: At top level: drivers/gpu//drm/msm/disp/dpu1/dpu_kms.c:701:21: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types] .debugfs_init= dpu_kms_debugfs_init, ^~~~ drivers/gpu//drm/msm/disp/dpu1/dpu_kms.c:701:21: note: (near initialization for 'kms_funcs.debugfs_init') cc1: some warnings being treated as errors vim +/return +227 drivers/gpu//drm/msm/disp/dpu1/dpu_kms.c --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vt6656: change alignment to match parenthesis
Change indentation to match parentheses. This complies with the Linux
kernel coding style and improves readability.
Signed-off-by: Benjamin Sherman
---
drivers/staging/vt6656/rxtx.c| 10 +-
drivers/staging/vt6656/usbpipe.c | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/vt6656/rxtx.c b/drivers/staging/vt6656/rxtx.c
index 9def0748ffee..4e9cfacf75f2 100644
--- a/drivers/staging/vt6656/rxtx.c
+++ b/drivers/staging/vt6656/rxtx.c
@@ -287,12 +287,12 @@ static u16 vnt_rxtx_datahead_g(struct
vnt_usb_send_context *tx_context,
buf->duration_a = vnt_get_duration_le(priv,
tx_context->pkt_type, need_ack);
buf->duration_b = vnt_get_duration_le(priv,
- PK_TYPE_11B, need_ack);
+ PK_TYPE_11B, need_ack);
}
buf->time_stamp_off_a = vnt_time_stamp_off(priv, rate);
buf->time_stamp_off_b = vnt_time_stamp_off(priv,
- priv->top_cck_basic_rate);
+ priv->top_cck_basic_rate);
tx_context->tx_hdr_size = vnt_mac_hdr_pos(tx_context, >hdr);
@@ -325,7 +325,7 @@ static u16 vnt_rxtx_datahead_g_fb(struct
vnt_usb_send_context *tx_context,
buf->time_stamp_off_a = vnt_time_stamp_off(priv, rate);
buf->time_stamp_off_b = vnt_time_stamp_off(priv,
- priv->top_cck_basic_rate);
+ priv->top_cck_basic_rate);
tx_context->tx_hdr_size = vnt_mac_hdr_pos(tx_context, >hdr);
@@ -655,7 +655,7 @@ static u16 vnt_rxtx_ab(struct vnt_usb_send_context
*tx_context,
u8 need_ack = tx_context->need_ack;
buf->rrv_time = vnt_rxtx_rsvtime_le16(priv, tx_context->pkt_type,
- frame_len, current_rate, need_ack);
+ frame_len, current_rate,
need_ack);
if (need_mic)
head = _head->tx_ab.tx.mic.head;
@@ -1036,7 +1036,7 @@ static int vnt_beacon_xmit(struct vnt_private *priv,
struct sk_buff *skb)
/* Get Duration and TimeStampOff */
short_head->duration = vnt_get_duration_le(priv,
- PK_TYPE_11B, false);
+ PK_TYPE_11B, false);
short_head->time_stamp_off =
vnt_time_stamp_off(priv, current_rate);
}
diff --git a/drivers/staging/vt6656/usbpipe.c b/drivers/staging/vt6656/usbpipe.c
index ff351a7a0876..d3304df6bd53 100644
--- a/drivers/staging/vt6656/usbpipe.c
+++ b/drivers/staging/vt6656/usbpipe.c
@@ -216,7 +216,7 @@ static void vnt_submit_rx_urb_complete(struct urb *urb)
}
urb->transfer_buffer = skb_put(rcb->skb,
- skb_tailroom(rcb->skb));
+ skb_tailroom(rcb->skb));
}
if (usb_submit_urb(urb, GFP_ATOMIC)) {
--
2.22.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/2] Staging: fbtft: Fix probing of gpio descriptor
Conversion to use gpio descriptors broke all gpio lookups as
devm_gpiod_get_index was converted to use dev->driver->name for
the gpio name lookup. Fix this by using the name param. In
addition gpiod_get post-fixes the -gpios to the name so that
shouldn't be included in the call. However this then breaks the
of_find_property call to see if the gpio entry exists as all
fbtft treats all gpios as optional. So use devm_gpiod_get_index_optional
instead which achieves the same thing and is simpler.
Nishad confirmed the changes where only ever compile tested.
Fixes: c440eee1a7a1 ("Staging: fbtft: Switch to the gpio descriptor interface")
Reviewed-by: Nicolas Saenz Julienne
Tested-by: Nicolas Saenz Julienne
Tested-by: Jan Sebastian Götte
Signed-off-by: Phil Reid
---
drivers/staging/fbtft/fbtft-core.c | 39 ++
1 file changed, 18 insertions(+), 21 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c
b/drivers/staging/fbtft/fbtft-core.c
index 9b07bad..44b8074 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -76,21 +76,18 @@ static int fbtft_request_one_gpio(struct fbtft_par *par,
struct gpio_desc **gpiop)
{
struct device *dev = par->info->device;
- struct device_node *node = dev->of_node;
int ret = 0;
- if (of_find_property(node, name, NULL)) {
- *gpiop = devm_gpiod_get_index(dev, dev->driver->name, index,
- GPIOD_OUT_HIGH);
- if (IS_ERR(*gpiop)) {
- ret = PTR_ERR(*gpiop);
- dev_err(dev,
- "Failed to request %s GPIO:%d\n", name, ret);
- return ret;
- }
- fbtft_par_dbg(DEBUG_REQUEST_GPIOS, par, "%s: '%s' GPIO\n",
- __func__, name);
+ *gpiop = devm_gpiod_get_index_optional(dev, name, index,
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(*gpiop)) {
+ ret = PTR_ERR(*gpiop);
+ dev_err(dev,
+ "Failed to request %s GPIO: %d\n", name, ret);
+ return ret;
}
+ fbtft_par_dbg(DEBUG_REQUEST_GPIOS, par, "%s: '%s' GPIO\n",
+ __func__, name);
return ret;
}
@@ -103,34 +100,34 @@ static int fbtft_request_gpios_dt(struct fbtft_par *par)
if (!par->info->device->of_node)
return -EINVAL;
- ret = fbtft_request_one_gpio(par, "reset-gpios", 0, >gpio.reset);
+ ret = fbtft_request_one_gpio(par, "reset", 0, >gpio.reset);
if (ret)
return ret;
- ret = fbtft_request_one_gpio(par, "dc-gpios", 0, >gpio.dc);
+ ret = fbtft_request_one_gpio(par, "dc", 0, >gpio.dc);
if (ret)
return ret;
- ret = fbtft_request_one_gpio(par, "rd-gpios", 0, >gpio.rd);
+ ret = fbtft_request_one_gpio(par, "rd", 0, >gpio.rd);
if (ret)
return ret;
- ret = fbtft_request_one_gpio(par, "wr-gpios", 0, >gpio.wr);
+ ret = fbtft_request_one_gpio(par, "wr", 0, >gpio.wr);
if (ret)
return ret;
- ret = fbtft_request_one_gpio(par, "cs-gpios", 0, >gpio.cs);
+ ret = fbtft_request_one_gpio(par, "cs", 0, >gpio.cs);
if (ret)
return ret;
- ret = fbtft_request_one_gpio(par, "latch-gpios", 0, >gpio.latch);
+ ret = fbtft_request_one_gpio(par, "latch", 0, >gpio.latch);
if (ret)
return ret;
for (i = 0; i < 16; i++) {
- ret = fbtft_request_one_gpio(par, "db-gpios", i,
+ ret = fbtft_request_one_gpio(par, "db", i,
>gpio.db[i]);
if (ret)
return ret;
- ret = fbtft_request_one_gpio(par, "led-gpios", i,
+ ret = fbtft_request_one_gpio(par, "led", i,
>gpio.led[i]);
if (ret)
return ret;
- ret = fbtft_request_one_gpio(par, "aux-gpios", i,
+ ret = fbtft_request_one_gpio(par, "aux", i,
>gpio.aux[i]);
if (ret)
return ret;
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/2] Staging: fbtft: Fix reset assertion when using gpio descriptor
Typically gpiod_set_value calls would assert the reset line and
then release it using the symantics of:
gpiod_set_value(par->gpio.reset, 0);
... delay
gpiod_set_value(par->gpio.reset, 1);
And the gpio binding would specify the polarity.
Prior to conversion to gpiod calls the polarity in the DT
was ignored and assumed to be active low. Fix it so that
DT polarity is respected.
Fixes: c440eee1a7a1 ("Staging: fbtft: Switch to the gpio descriptor interface")
Reviewed-by: Nicolas Saenz Julienne
Tested-by: Nicolas Saenz Julienne
Tested-by: Jan Sebastian Götte
Signed-off-by: Phil Reid
---
drivers/staging/fbtft/fbtft-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c
b/drivers/staging/fbtft/fbtft-core.c
index 44b8074..bc75025 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -231,9 +231,9 @@ static void fbtft_reset(struct fbtft_par *par)
if (!par->gpio.reset)
return;
fbtft_par_dbg(DEBUG_RESET, par, "%s()\n", __func__);
- gpiod_set_value_cansleep(par->gpio.reset, 0);
- usleep_range(20, 40);
gpiod_set_value_cansleep(par->gpio.reset, 1);
+ usleep_range(20, 40);
+ gpiod_set_value_cansleep(par->gpio.reset, 0);
msleep(120);
}
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/2] Staging: fbtft: Fix probing of gpio descriptor
GPIO probing and reset polarity are broken.
Fix them.
Fixes: c440eee1a7a1 ("Staging: fbtft: Switch to the gpio descriptor interface")
Changes from v2:
- Add fixes tag to "Fix reset assertion when using gpio descriptor"
- Add tested-by / reviewed-by tags
Phil Reid (2):
Staging: fbtft: Fix probing of gpio descriptor
Staging: fbtft: Fix reset assertion when using gpio descriptor
drivers/staging/fbtft/fbtft-core.c | 43 ++
1 file changed, 20 insertions(+), 23 deletions(-)
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: kpc2000: whitespace and line length cleanup
On Mon, 2019-07-15 at 14:21 -0700, john.hubb...@gmail.com wrote:
> From: John Hubbard
>
> This commit was created by running indent(1):
> `indent -linux`
>
> ...and then applying some manual corrections and
> cleanup afterward, to keep it sane. No functional changes
> were made.
I don't find many of these whitespace changes "better".
Sometimes, it's just fine to have > 80 char lines.
Alignment formatting was OK before this and now has
many odd uses that make reading for a human harder
rather than simpler or easier.
> diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c
> b/drivers/staging/kpc2000/kpc2000_i2c.c
[]
> @@ -33,9 +33,9 @@ MODULE_LICENSE("GPL");
> MODULE_AUTHOR("matt.sick...@daktronics.com");
>
> struct i2c_device {
> - unsigned long smba;
> - struct i2c_adapter adapter;
> - unsigned intfeatures;
> + unsigned long smba;
> + struct i2c_adapter adapter;
> + unsigned intfeatures;
Here the spaces before the identifier are converted to tab aligned
> };
>
> /*
> @@ -52,9 +52,9 @@ struct i2c_device {
> #define SMBHSTDAT0(p) ((5 * REG_SIZE) + (p)->smba)
> #define SMBHSTDAT1(p) ((6 * REG_SIZE) + (p)->smba)
> #define SMBBLKDAT(p)((7 * REG_SIZE) + (p)->smba)
> -#define SMBPEC(p) ((8 * REG_SIZE) + (p)->smba) /* ICH3 and later */
> -#define SMBAUXSTS(p)((12 * REG_SIZE) + (p)->smba) /* ICH4 and later */
> -#define SMBAUXCTL(p)((13 * REG_SIZE) + (p)->smba) /* ICH4 and later */
> +#define SMBPEC(p) ((8 * REG_SIZE) + (p)->smba)/* ICH3 and
> later */
> +#define SMBAUXSTS(p)((12 * REG_SIZE) + (p)->smba)/* ICH4 and
> later */
> +#define SMBAUXCTL(p)((13 * REG_SIZE) + (p)->smba)/* ICH4 and
> later */
But here the #define value still has spaces but the comment uses tabs.
Why tab align the comments but not the #define value?
> @@ -136,17 +138,21 @@ static int i801_check_pre(struct i2c_device *priv)
[]
> status &= STATUS_FLAGS;
> if (status) {
> - //dev_dbg(>adapter.dev, "Clearing status flags (%02x)\n",
> status);
> + //dev_dbg(>adapter.dev,
> + //"Clearing status flags (%02x)\n", status);
This was better before.
An improvement might be to add more macros like:
#define i2c_err(priv, fmt, ...)
dev_err(&(priv)->adapter.dev, fmt, ##__VA_ARGS__)
#define i2c_dbg(priv, fmt, ...)
dev_dbg(&(priv)->adapter.dev, fmt, ##__VA_ARGS__)
So all these uses of dev_(>adapter.dev, ...)
become much shorter visually and the line wrapping becomes
rather better.
> outb_p(status, SMBHSTSTS(priv));
> status = inb_p(SMBHSTSTS(priv)) & STATUS_FLAGS;
> if (status) {
> - dev_err(>adapter.dev, "Failed clearing status
> flags (%02x)\n", status);
> + dev_err(>adapter.dev,
> + "Failed clearing status flags (%02x)\n",
> + status);
e.g.:
i2c_err(priv, "Failed clearing status flags (%02x)\n",
status);
etc...
[]
> @@ -301,7 +322,8 @@ static int i801_block_transaction_byte_by_byte(struct
> i2c_device *priv, union i2
> else
> smbcmd = I801_BLOCK_LAST;
> } else {
> - if (command == I2C_SMBUS_I2C_BLOCK_DATA && read_write
> == I2C_SMBUS_READ)
> + if (command == I2C_SMBUS_I2C_BLOCK_DATA
> + && read_write == I2C_SMBUS_READ)
logic continuations should be at EOL.
if (command == I2C_SMBUS_I2C_BLOCK_DATA &&
read_write == I2C_SMBUS_READ)
[]
> @@ -558,13 +614,14 @@ static u32 i801_func(struct i2c_adapter *adapter)
> I2C_FUNC_SMBUS_WORD_DATA | /* _READ_WORD_DATA
> _WRITE_WORD_DATA */
> I2C_FUNC_SMBUS_BLOCK_DATA| /* _READ_BLOCK_DATA
> _WRITE_BLOCK_DATA */
> !I2C_FUNC_SMBUS_I2C_BLOCK| /* _READ_I2C_BLOCK
> _WRITE_I2C_BLOCK */
> - !I2C_FUNC_SMBUS_EMUL; /* _QUICK _BYTE _BYTE_DATA
> _WORD_DATA _PROC_CALL _WRITE_BLOCK_DATA _I2C_BLOCK _PEC */
> + /* _QUICK _BYTE _BYTE_DATA _WORD_DATA _PROC_CALL
> _WRITE_BLOCK_DATA _I2C_BLOCK _PEC : */
> + !I2C_FUNC_SMBUS_EMUL;
> return f;
> }
>
> static const struct i2c_algorithm smbus_algorithm = {
> - .smbus_xfer = i801_access,
> - .functionality = i801_func,
> + .smbus_xfer = i801_access,
> + .functionality = i801_func,
Many people prefer the aligned function names.
etc...
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: kpc2000: whitespace and line length cleanup
On 7/15/19 3:21 PM, Joe Perches wrote:
> On Mon, 2019-07-15 at 14:21 -0700, john.hubb...@gmail.com wrote:
>> From: John Hubbard
>>
>> This commit was created by running indent(1):
>> `indent -linux`
>>
>> ...and then applying some manual corrections and
>> cleanup afterward, to keep it sane. No functional changes
>> were made.
>
> I don't find many of these whitespace changes "better".
>
> Sometimes, it's just fine to have > 80 char lines.
Definitely agree! :)
>
> Alignment formatting was OK before this and now has
> many odd uses that make reading for a human harder
> rather than simpler or easier.
OK, I'll accept that. I attempted to pick something that fit
on the screen [much!] better, without making it less readable, but if
people feel that it is a net "worse", then let's just drop
the patch, no problem.
>
>> diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c
>> b/drivers/staging/kpc2000/kpc2000_i2c.c
> []
>> @@ -33,9 +33,9 @@ MODULE_LICENSE("GPL");
>> MODULE_AUTHOR("matt.sick...@daktronics.com");
>>
>> struct i2c_device {
>> -unsigned long smba;
>> -struct i2c_adapter adapter;
>> -unsigned intfeatures;
>> +unsigned long smba;
>> +struct i2c_adapter adapter;
>> +unsigned intfeatures;
>
> Here the spaces before the identifier are converted to tab aligned
>
>> };
>>
>> /*
>> @@ -52,9 +52,9 @@ struct i2c_device {
>> #define SMBHSTDAT0(p) ((5 * REG_SIZE) + (p)->smba)
>> #define SMBHSTDAT1(p) ((6 * REG_SIZE) + (p)->smba)
>> #define SMBBLKDAT(p)((7 * REG_SIZE) + (p)->smba)
>> -#define SMBPEC(p) ((8 * REG_SIZE) + (p)->smba) /* ICH3 and later */
>> -#define SMBAUXSTS(p)((12 * REG_SIZE) + (p)->smba) /* ICH4 and later */
>> -#define SMBAUXCTL(p)((13 * REG_SIZE) + (p)->smba) /* ICH4 and later */
>> +#define SMBPEC(p) ((8 * REG_SIZE) + (p)->smba) /* ICH3 and
>> later */
>> +#define SMBAUXSTS(p)((12 * REG_SIZE) + (p)->smba) /* ICH4 and
>> later */
>> +#define SMBAUXCTL(p)((13 * REG_SIZE) + (p)->smba) /* ICH4 and
>> later */
>
> But here the #define value still has spaces but the comment uses tabs.
> Why tab align the comments but not the #define value?
>
>> @@ -136,17 +138,21 @@ static int i801_check_pre(struct i2c_device *priv)
> []
>> status &= STATUS_FLAGS;
>> if (status) {
>> -//dev_dbg(>adapter.dev, "Clearing status flags (%02x)\n",
>> status);
>> +//dev_dbg(>adapter.dev,
>> +//"Clearing status flags (%02x)\n", status);
>
> This was better before.
>
> An improvement might be to add more macros like:
>
> #define i2c_err(priv, fmt, ...)
> dev_err(&(priv)->adapter.dev, fmt, ##__VA_ARGS__)
> #define i2c_dbg(priv, fmt, ...)
> dev_dbg(&(priv)->adapter.dev, fmt, ##__VA_ARGS__)
>
> So all these uses of dev_(>adapter.dev, ...)
> become much shorter visually and the line wrapping becomes
> rather better.
>
>> outb_p(status, SMBHSTSTS(priv));
>> status = inb_p(SMBHSTSTS(priv)) & STATUS_FLAGS;
>> if (status) {
>> -dev_err(>adapter.dev, "Failed clearing status
>> flags (%02x)\n", status);
>> +dev_err(>adapter.dev,
>> +"Failed clearing status flags (%02x)\n",
>> +status);
>
> e.g.:
> i2c_err(priv, "Failed clearing status flags (%02x)\n",
> status);
>
> etc...
>
>
> []
>
>> @@ -301,7 +322,8 @@ static int i801_block_transaction_byte_by_byte(struct
>> i2c_device *priv, union i2
>> else
>> smbcmd = I801_BLOCK_LAST;
>> } else {
>> -if (command == I2C_SMBUS_I2C_BLOCK_DATA && read_write
>> == I2C_SMBUS_READ)
>> +if (command == I2C_SMBUS_I2C_BLOCK_DATA
>> +&& read_write == I2C_SMBUS_READ)
>
> logic continuations should be at EOL.
>
> if (command == I2C_SMBUS_I2C_BLOCK_DATA &&
> read_write == I2C_SMBUS_READ)
>
> []
>> @@ -558,13 +614,14 @@ static u32 i801_func(struct i2c_adapter *adapter)
>> I2C_FUNC_SMBUS_WORD_DATA | /* _READ_WORD_DATA
>> _WRITE_WORD_DATA */
>> I2C_FUNC_SMBUS_BLOCK_DATA| /* _READ_BLOCK_DATA
>> _WRITE_BLOCK_DATA */
>> !I2C_FUNC_SMBUS_I2C_BLOCK| /* _READ_I2C_BLOCK
>> _WRITE_I2C_BLOCK */
>> -!I2C_FUNC_SMBUS_EMUL; /* _QUICK _BYTE _BYTE_DATA
>> _WORD_DATA _PROC_CALL _WRITE_BLOCK_DATA _I2C_BLOCK _PEC */
>> +/* _QUICK _BYTE _BYTE_DATA _WORD_DATA _PROC_CALL
>> _WRITE_BLOCK_DATA _I2C_BLOCK _PEC : */
>> +!I2C_FUNC_SMBUS_EMUL;
>> return f;
>> }
>>
>> static const struct i2c_algorithm smbus_algorithm = {
>> -.smbus_xfer = i801_access,
>> -
Re: [PATCH] staging: kpc2000: Convert put_page() to put_user_page*()
On 7/15/19 3:01 PM, John Hubbard wrote:
> On 7/15/19 2:47 PM, Matt Sickler wrote:
...
> I agree: the PageReserved check looks unnecessary here, from my
> outside-the-kpc_2000-team
> perspective, anyway. Assuming that your analysis above is correct, you could
> collapse that
> whole think into just:
>
> @@ -211,17 +209,8 @@ void transfer_complete_cb(struct aio_cb_data *acd,
> size_t xfr_count, u32 flags)
> BUG_ON(acd->ldev == NULL);
> BUG_ON(acd->ldev->pldev == NULL);
>
> - for (i = 0 ; i < acd->page_count ; i++) {
> - if (!PageReserved(acd->user_pages[i])) {
> - set_page_dirty(acd->user_pages[i]);
> - }
> - }
> -
> dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents,
> acd->ldev->dir);
> -
> - for (i = 0 ; i < acd->page_count ; i++) {
> - put_page(acd->user_pages[i]);
> - }
> + put_user_pages_dirty(>user_pages[i], acd->page_count);
Ahem, I typed too quickly. :) Please make that:
put_user_pages_dirty(acd->user_pages, acd->page_count);
thanks,
--
John Hubbard
NVIDIA
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] staging: kpc2000: Convert put_page() to put_user_page*()
It looks like Outlook is going to absolutely trash this email. Hopefully it
comes through okay.
>> There have been issues with get_user_pages and filesystem writeback.
>> The issues are better described in [1].
>>
>> The solution being proposed wants to keep track of gup_pinned pages
>which will allow to take furthur steps to coordinate between subsystems
>using gup.
>>
>> put_user_page() simply calls put_page inside for now. But the
>implementation will change once all call sites of put_page() are converted.
>>
>> I currently do not have the driver to test. Could I have some suggestions to
>test this code? The solution is currently implemented in [2] and
>> it would be great if we could apply the patch on top of [2] and run some
>tests to check if any regressions occur.
>
>Because this is a common pattern, and because the code here doesn't likely
>need to set page dirty before the dma_unmap_sg call, I think the following
>would be better (it's untested), instead of the above diff hunk:
>
>diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c
>b/drivers/staging/kpc2000/kpc_dma/fileops.c
>index 48ca88bc6b0b..d486f9866449 100644
>--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
>+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
>@@ -211,16 +211,13 @@ void transfer_complete_cb(struct aio_cb_data
>*acd, size_t xfr_count, u32 flags)
>BUG_ON(acd->ldev == NULL);
>BUG_ON(acd->ldev->pldev == NULL);
>
>- for (i = 0 ; i < acd->page_count ; i++) {
>- if (!PageReserved(acd->user_pages[i])) {
>- set_page_dirty(acd->user_pages[i]);
>- }
>- }
>-
>dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents,
> acd->ldev->dir);
>
>for (i = 0 ; i < acd->page_count ; i++) {
>- put_page(acd->user_pages[i]);
>+ if (!PageReserved(acd->user_pages[i])) {
>+ put_user_pages_dirty(>user_pages[i], 1);
>+ else
>+ put_user_page(acd->user_pages[i]);
>}
>
>sg_free_table(>sgt);
I don't think I ever really knew the right way to do this.
The changes Bharath suggested look okay to me. I'm not sure about the check
for PageReserved(), though. At first glance it appears to be equivalent to
what was there before, but maybe I should learn what that Reserved page flag
really means.
>From [1], the only comment that seems applicable is
* - MMIO/DMA pages. Some architectures don't allow to ioremap pages that are
* not marked PG_reserved (as they might be in use by somebody else who does
* not respect the caching strategy).
These pages should be coming from anonymous (RAM, not file backed) memory in
userspace. Sometimes it comes from hugepage backed memory, though I don't
think that makes a difference. I should note that transfer_complete_cb handles
both RAM to device and device to RAM DMAs, if that matters.
[1] https://elixir.bootlin.com/linux/v5.2/source/include/linux/page-flags.h#L17
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: kpc2000: Convert put_page() to put_user_page*()
On 7/15/19 2:47 PM, Matt Sickler wrote:
> It looks like Outlook is going to absolutely trash this email. Hopefully it
> comes through okay.
>
...
>>
>> Because this is a common pattern, and because the code here doesn't likely
>> need to set page dirty before the dma_unmap_sg call, I think the following
>> would be better (it's untested), instead of the above diff hunk:
>>
>> diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c
>> b/drivers/staging/kpc2000/kpc_dma/fileops.c
>> index 48ca88bc6b0b..d486f9866449 100644
>> --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
>> +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
>> @@ -211,16 +211,13 @@ void transfer_complete_cb(struct aio_cb_data
>> *acd, size_t xfr_count, u32 flags)
>>BUG_ON(acd->ldev == NULL);
>>BUG_ON(acd->ldev->pldev == NULL);
>>
>> - for (i = 0 ; i < acd->page_count ; i++) {
>> - if (!PageReserved(acd->user_pages[i])) {
>> - set_page_dirty(acd->user_pages[i]);
>> - }
>> - }
>> -
>>dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents,
>> acd->ldev->dir);
>>
>>for (i = 0 ; i < acd->page_count ; i++) {
>> - put_page(acd->user_pages[i]);
>> + if (!PageReserved(acd->user_pages[i])) {
>> + put_user_pages_dirty(>user_pages[i], 1);
>> + else
>> + put_user_page(acd->user_pages[i]);
>>}
>>
>>sg_free_table(>sgt);
>
> I don't think I ever really knew the right way to do this.
>
> The changes Bharath suggested look okay to me. I'm not sure about the check
> for PageReserved(), though. At first glance it appears to be equivalent to
> what was there before, but maybe I should learn what that Reserved page flag
> really means.
> From [1], the only comment that seems applicable is
> * - MMIO/DMA pages. Some architectures don't allow to ioremap pages that are
> * not marked PG_reserved (as they might be in use by somebody else who does
> * not respect the caching strategy).
>
> These pages should be coming from anonymous (RAM, not file backed) memory in
> userspace. Sometimes it comes from hugepage backed memory, though I don't
> think that makes a difference. I should note that transfer_complete_cb
> handles both RAM to device and device to RAM DMAs, if that matters.
>
> [1]
> https://elixir.bootlin.com/linux/v5.2/source/include/linux/page-flags.h#L17
>
I agree: the PageReserved check looks unnecessary here, from my
outside-the-kpc_2000-team
perspective, anyway. Assuming that your analysis above is correct, you could
collapse that
whole think into just:
@@ -211,17 +209,8 @@ void transfer_complete_cb(struct aio_cb_data *acd, size_t
xfr_count, u32 flags)
BUG_ON(acd->ldev == NULL);
BUG_ON(acd->ldev->pldev == NULL);
- for (i = 0 ; i < acd->page_count ; i++) {
- if (!PageReserved(acd->user_pages[i])) {
- set_page_dirty(acd->user_pages[i]);
- }
- }
-
dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents,
acd->ldev->dir);
-
- for (i = 0 ; i < acd->page_count ; i++) {
- put_page(acd->user_pages[i]);
- }
+ put_user_pages_dirty(>user_pages[i], acd->page_count);
sg_free_table(>sgt);
(Also, Matt, I failed to Cc: you on a semi-related cleanup that I just sent out
for this
driver, as long as I have your attention:
https://lore.kernel.org/r/20190715212123.432-1-jhubb...@nvidia.com
)
thanks,
--
John Hubbard
NVIDIA
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: kpc2000: Convert put_page() to put_user_page*()
There have been issues with get_user_pages and filesystem writeback.
The issues are better described in [1].
The solution being proposed wants to keep track of gup_pinned pages
which will allow to take furthur steps to coordinate between subsystems
using gup.
put_user_page() simply calls put_page inside for now. But the
implementation will change once all call sites of put_page() are
converted.
[1] https://lwn.net/Articles/753027/
Cc: Matt Sickler
Cc: Greg Kroah-Hartman
Cc: Jérôme Glisse
Cc: Ira Weiny
Cc: John Hubbard
Cc: linux...@kvack.org
Cc: de...@driverdev.osuosl.org
Reviewed-by: John Hubbard
Signed-off-by: Bharath Vedartham
---
Changes since v1
- Added John's reviewed-by tag
- Moved para talking about testing below
the '---'
- Moved logic of set_page_diry below dma_unmap_sg
as per John's suggestion
I currently do not have the driver to test. Could I have some
suggestions to test this code? The solution is currently implemented
in https://github.com/johnhubbard/linux/tree/gup_dma_core and it would be great
if we could apply the patch on top of the repo and run some
tests to check if any regressions occur.
---
drivers/staging/kpc2000/kpc_dma/fileops.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c
b/drivers/staging/kpc2000/kpc_dma/fileops.c
index 48ca88b..3d1a00a 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -190,9 +190,7 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
sg_free_table(>sgt);
err_dma_map_sg:
err_alloc_sg_table:
- for (i = 0 ; i < acd->page_count ; i++) {
- put_page(acd->user_pages[i]);
- }
+ put_user_pages(acd->user_pages, acd->page_count);
err_get_user_pages:
kfree(acd->user_pages);
err_alloc_userpages:
@@ -211,16 +209,13 @@ void transfer_complete_cb(struct aio_cb_data *acd,
size_t xfr_count, u32 flags)
BUG_ON(acd->ldev == NULL);
BUG_ON(acd->ldev->pldev == NULL);
- for (i = 0 ; i < acd->page_count ; i++) {
- if (!PageReserved(acd->user_pages[i])) {
- set_page_dirty(acd->user_pages[i]);
- }
- }
-
dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents,
acd->ldev->dir);
- for (i = 0 ; i < acd->page_count ; i++) {
- put_page(acd->user_pages[i]);
+ for (i = 0; i < acd->page_count; i++) {
+ if (!PageReserved(acd->user_pages[i]))
+ put_user_pages_dirty(>user_pages[i], 1);
+ else
+ put_user_page(acd->user_pages[i]);
}
sg_free_table(>sgt);
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: kpc2000: whitespace and line length cleanup
From: John Hubbard
This commit was created by running indent(1):
`indent -linux`
...and then applying some manual corrections and
cleanup afterward, to keep it sane. No functional changes
were made.
In addition to whitespace changes, some strings were split,
but not strings that were likely to be a grep problem
(in other words, if a user is likely to grep for a string
within the driver, that should still work in most cases).
A few "void * foo" cases were fixed to be "void *foo".
That was enough to make checkpatch.pl run without errors,
although note that there are lots of serious warnings
remaining--but those require functional, not just whitespace
changes. So those are left for a separate patch.
Cc: Greg Kroah-Hartman
Cc: Simon Sandström
Cc: Geordan Neukum
Cc: Jeremy Sowden
Cc: Dan Carpenter
Cc: Vandana BN
Cc: de...@driverdev.osuosl.org
Cc: Bharath Vedartham
Signed-off-by: John Hubbard
---
drivers/staging/kpc2000/kpc2000_i2c.c | 189 +++--
drivers/staging/kpc2000/kpc2000_spi.c | 116 +-
drivers/staging/kpc2000/kpc_dma/dma.c | 109 ++
drivers/staging/kpc2000/kpc_dma/fileops.c | 199 +++---
.../staging/kpc2000/kpc_dma/kpc_dma_driver.c | 113 +-
.../staging/kpc2000/kpc_dma/kpc_dma_driver.h | 156 +++---
6 files changed, 509 insertions(+), 373 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c
b/drivers/staging/kpc2000/kpc2000_i2c.c
index b108da4ac633..93fa1858f6b5 100644
--- a/drivers/staging/kpc2000/kpc2000_i2c.c
+++ b/drivers/staging/kpc2000/kpc2000_i2c.c
@@ -33,9 +33,9 @@ MODULE_LICENSE("GPL");
MODULE_AUTHOR("matt.sick...@daktronics.com");
struct i2c_device {
- unsigned long smba;
- struct i2c_adapter adapter;
- unsigned intfeatures;
+ unsigned long smba;
+ struct i2c_adapter adapter;
+ unsigned intfeatures;
};
/*
@@ -52,9 +52,9 @@ struct i2c_device {
#define SMBHSTDAT0(p) ((5 * REG_SIZE) + (p)->smba)
#define SMBHSTDAT1(p) ((6 * REG_SIZE) + (p)->smba)
#define SMBBLKDAT(p)((7 * REG_SIZE) + (p)->smba)
-#define SMBPEC(p) ((8 * REG_SIZE) + (p)->smba) /* ICH3 and later */
-#define SMBAUXSTS(p)((12 * REG_SIZE) + (p)->smba) /* ICH4 and later */
-#define SMBAUXCTL(p)((13 * REG_SIZE) + (p)->smba) /* ICH4 and later */
+#define SMBPEC(p) ((8 * REG_SIZE) + (p)->smba) /* ICH3 and later */
+#define SMBAUXSTS(p)((12 * REG_SIZE) + (p)->smba) /* ICH4 and later */
+#define SMBAUXCTL(p)((13 * REG_SIZE) + (p)->smba) /* ICH4 and later */
/* PCI Address Constants */
#define SMBBAR 4
@@ -74,20 +74,20 @@ struct i2c_device {
/* Other settings */
#define MAX_RETRIES 400
-#define ENABLE_INT9 0 /* set to 0x01 to enable - untested */
+#define ENABLE_INT9 0 /* set to 0x01 to enable - untested */
/* I801 command constants */
#define I801_QUICK 0x00
#define I801_BYTE 0x04
#define I801_BYTE_DATA 0x08
#define I801_WORD_DATA 0x0C
-#define I801_PROC_CALL 0x10/* unimplemented */
+#define I801_PROC_CALL 0x10 /* unimplemented */
#define I801_BLOCK_DATA 0x14
-#define I801_I2C_BLOCK_DATA 0x18/* ICH5 and later */
+#define I801_I2C_BLOCK_DATA 0x18 /* ICH5 and later */
#define I801_BLOCK_LAST 0x34
-#define I801_I2C_BLOCK_LAST 0x38/* ICH5 and later */
+#define I801_I2C_BLOCK_LAST 0x38 /* ICH5 and later */
#define I801_START 0x40
-#define I801_PEC_EN 0x80/* ICH3 and later */
+#define I801_PEC_EN 0x80 /* ICH3 and later */
/* I801 Hosts Status register bits */
#define SMBHSTSTS_BYTE_DONE 0x80
@@ -99,7 +99,9 @@ struct i2c_device {
#define SMBHSTSTS_INTR 0x02
#define SMBHSTSTS_HOST_BUSY 0x01
-#define STATUS_FLAGS(SMBHSTSTS_BYTE_DONE | SMBHSTSTS_FAILED |
SMBHSTSTS_BUS_ERR | SMBHSTSTS_DEV_ERR | SMBHSTSTS_INTR)
+#define STATUS_FLAGS \
+ (SMBHSTSTS_BYTE_DONE | SMBHSTSTS_FAILED | SMBHSTSTS_BUS_ERR | \
+SMBHSTSTS_DEV_ERR | SMBHSTSTS_INTR)
/* Older devices have their ID defined in */
#define PCI_DEVICE_ID_INTEL_COUGARPOINT_SMBUS 0x1c22
@@ -136,17 +138,21 @@ static int i801_check_pre(struct i2c_device *priv)
status = inb_p(SMBHSTSTS(priv));
if (status & SMBHSTSTS_HOST_BUSY) {
- dev_err(>adapter.dev, "SMBus is busy, can't use it!
(status=%x)\n", status);
+ dev_err(>adapter.dev,
+ "SMBus is busy, can't use it! (status=%x)\n", status);
return -EBUSY;
}
status &= STATUS_FLAGS;
if (status) {
- //dev_dbg(>adapter.dev, "Clearing status flags (%02x)\n",
status);
+ //dev_dbg(>adapter.dev,
+ //"Clearing status flags (%02x)\n", status);
[PATCH 0/1] staging: kpc2000: whitespace and line length cleanup
From: John Hubbard
Hi everyone,
This is an easy, drive-by cleanup that I did while reviewing Bharath's
changes to convert over to put_user_page(). It should make the code less
obviously non-conforming, and therefore help future reviews and cleanups.
This is on top of latest linux.git, commit fec88ab0af97 ("Merge tag
'for-linus-hmm' of git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma"),
and it does NOT have Bharath's patch applied, so it conflicts (but should
be trivial to resolve, regardless of which is applied first, as it's just
whitespace).
Cc: Greg Kroah-Hartman
Cc: Simon Sandström
Cc: Geordan Neukum
Cc: Jeremy Sowden
Cc: Dan Carpenter
Cc: Vandana BN
Cc: de...@driverdev.osuosl.org
Cc: Bharath Vedartham
John Hubbard (1):
staging: kpc2000: whitespace and line length cleanup
drivers/staging/kpc2000/kpc2000_i2c.c | 189 +++--
drivers/staging/kpc2000/kpc2000_spi.c | 116 +-
drivers/staging/kpc2000/kpc_dma/dma.c | 109 ++
drivers/staging/kpc2000/kpc_dma/fileops.c | 199 +++---
.../staging/kpc2000/kpc_dma/kpc_dma_driver.c | 113 +-
.../staging/kpc2000/kpc_dma/kpc_dma_driver.h | 154 +++---
6 files changed, 507 insertions(+), 373 deletions(-)
--
2.22.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: kpc2000: Convert put_page() to put_user_page*()
On Mon, Jul 15, 2019 at 01:14:13PM -0700, John Hubbard wrote:
> On 7/15/19 12:52 PM, Bharath Vedartham wrote:
> > There have been issues with get_user_pages and filesystem writeback.
> > The issues are better described in [1].
> >
> > The solution being proposed wants to keep track of gup_pinned pages which
> > will allow to take furthur steps to coordinate between subsystems using gup.
> >
> > put_user_page() simply calls put_page inside for now. But the
> > implementation will change once all call sites of put_page() are converted.
> >
> > I currently do not have the driver to test. Could I have some suggestions
> > to test this code? The solution is currently implemented in [2] and
> > it would be great if we could apply the patch on top of [2] and run some
> > tests to check if any regressions occur.
>
> Hi Bharath,
>
> Process point: the above paragraph, and other meta-questions (about the
> patch, rather than part of the patch) should be placed either after the
> "---", or in a cover letter (git-send-email --cover-letter). That way, the
> patch itself is in a commit-able state.
>
> One more below:
Will fix that in the next version.
> >
> > [1] https://lwn.net/Articles/753027/
> > [2] https://github.com/johnhubbard/linux/tree/gup_dma_core
> >
> > Cc: Matt Sickler
> > Cc: Greg Kroah-Hartman
> > Cc: Jérôme Glisse
> > Cc: Ira Weiny
> > Cc: John Hubbard
> > Cc: linux...@kvack.org
> > Cc: de...@driverdev.osuosl.org
> >
> > Signed-off-by: Bharath Vedartham
> > ---
> > drivers/staging/kpc2000/kpc_dma/fileops.c | 8 ++--
> > 1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c
> > b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > index 6166587..82c70e6 100644
> > --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> > +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > @@ -198,9 +198,7 @@ int kpc_dma_transfer(struct dev_private_data *priv,
> > struct kiocb *kcb, unsigned
> > sg_free_table(>sgt);
> > err_dma_map_sg:
> > err_alloc_sg_table:
> > - for (i = 0 ; i < acd->page_count ; i++){
> > - put_page(acd->user_pages[i]);
> > - }
> > + put_user_pages(acd->user_pages, acd->page_count);
> > err_get_user_pages:
> > kfree(acd->user_pages);
> > err_alloc_userpages:
> > @@ -229,9 +227,7 @@ void transfer_complete_cb(struct aio_cb_data *acd,
> > size_t xfr_count, u32 flags)
> >
> > dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents,
> > acd->ldev->dir);
> >
> > - for (i = 0 ; i < acd->page_count ; i++){
> > - put_page(acd->user_pages[i]);
> > - }
> > + put_user_pages(acd->user_pages, acd->page_count);
> >
> > sg_free_table(>sgt);
> >
> >
>
> Because this is a common pattern, and because the code here doesn't likely
> need to set page dirty before the dma_unmap_sg call, I think the following
> would be better (it's untested), instead of the above diff hunk:
>
> diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c
> b/drivers/staging/kpc2000/kpc_dma/fileops.c
> index 48ca88bc6b0b..d486f9866449 100644
> --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> @@ -211,16 +211,13 @@ void transfer_complete_cb(struct aio_cb_data *acd,
> size_t xfr_count, u32 flags)
> BUG_ON(acd->ldev == NULL);
> BUG_ON(acd->ldev->pldev == NULL);
>
> - for (i = 0 ; i < acd->page_count ; i++) {
> - if (!PageReserved(acd->user_pages[i])) {
> - set_page_dirty(acd->user_pages[i]);
> - }
> - }
> -
> dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents,
> acd->ldev->dir);
>
> for (i = 0 ; i < acd->page_count ; i++) {
> - put_page(acd->user_pages[i]);
> + if (!PageReserved(acd->user_pages[i])) {
> + put_user_pages_dirty(>user_pages[i], 1);
> + else
> + put_user_page(acd->user_pages[i]);
> }
>
> sg_free_table(>sgt);
I had my doubts on this. This definitley needs to be looked at by the
driver author.
> Assuming that you make those two changes, you can add:
>
> Reviewed-by: John Hubbard
Great!
>
> thanks,
> --
> John Hubbard
> NVIDIA
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: kpc2000: Convert put_page() to put_user_page*()
On 7/15/19 12:52 PM, Bharath Vedartham wrote:
> There have been issues with get_user_pages and filesystem writeback.
> The issues are better described in [1].
>
> The solution being proposed wants to keep track of gup_pinned pages which
> will allow to take furthur steps to coordinate between subsystems using gup.
>
> put_user_page() simply calls put_page inside for now. But the implementation
> will change once all call sites of put_page() are converted.
>
> I currently do not have the driver to test. Could I have some suggestions to
> test this code? The solution is currently implemented in [2] and
> it would be great if we could apply the patch on top of [2] and run some
> tests to check if any regressions occur.
Hi Bharath,
Process point: the above paragraph, and other meta-questions (about the patch,
rather than part of the patch) should be placed either after the "---", or in a
cover letter (git-send-email --cover-letter). That way, the patch itself is in
a commit-able state.
One more below:
>
> [1] https://lwn.net/Articles/753027/
> [2] https://github.com/johnhubbard/linux/tree/gup_dma_core
>
> Cc: Matt Sickler
> Cc: Greg Kroah-Hartman
> Cc: Jérôme Glisse
> Cc: Ira Weiny
> Cc: John Hubbard
> Cc: linux...@kvack.org
> Cc: de...@driverdev.osuosl.org
>
> Signed-off-by: Bharath Vedartham
> ---
> drivers/staging/kpc2000/kpc_dma/fileops.c | 8 ++--
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c
> b/drivers/staging/kpc2000/kpc_dma/fileops.c
> index 6166587..82c70e6 100644
> --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> @@ -198,9 +198,7 @@ int kpc_dma_transfer(struct dev_private_data *priv,
> struct kiocb *kcb, unsigned
> sg_free_table(>sgt);
> err_dma_map_sg:
> err_alloc_sg_table:
> - for (i = 0 ; i < acd->page_count ; i++){
> - put_page(acd->user_pages[i]);
> - }
> + put_user_pages(acd->user_pages, acd->page_count);
> err_get_user_pages:
> kfree(acd->user_pages);
> err_alloc_userpages:
> @@ -229,9 +227,7 @@ void transfer_complete_cb(struct aio_cb_data *acd,
> size_t xfr_count, u32 flags)
>
> dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents,
> acd->ldev->dir);
>
> - for (i = 0 ; i < acd->page_count ; i++){
> - put_page(acd->user_pages[i]);
> - }
> + put_user_pages(acd->user_pages, acd->page_count);
>
> sg_free_table(>sgt);
>
>
Because this is a common pattern, and because the code here doesn't likely need
to set page dirty before the dma_unmap_sg call, I think the following would be
better (it's untested), instead of the above diff hunk:
diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c
b/drivers/staging/kpc2000/kpc_dma/fileops.c
index 48ca88bc6b0b..d486f9866449 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -211,16 +211,13 @@ void transfer_complete_cb(struct aio_cb_data *acd,
size_t xfr_count, u32 flags)
BUG_ON(acd->ldev == NULL);
BUG_ON(acd->ldev->pldev == NULL);
- for (i = 0 ; i < acd->page_count ; i++) {
- if (!PageReserved(acd->user_pages[i])) {
- set_page_dirty(acd->user_pages[i]);
- }
- }
-
dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents,
acd->ldev->dir);
for (i = 0 ; i < acd->page_count ; i++) {
- put_page(acd->user_pages[i]);
+ if (!PageReserved(acd->user_pages[i])) {
+ put_user_pages_dirty(>user_pages[i], 1);
+ else
+ put_user_page(acd->user_pages[i]);
}
sg_free_table(>sgt);
Assuming that you make those two changes, you can add:
Reviewed-by: John Hubbard
thanks,
--
John Hubbard
NVIDIA
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8712: Delete multiple blank line.
On Sat, Jul 13, 2019 at 06:57:43PM -0300, christianlucian...@gmail.com wrote: > From: Christian Luciano Moreno > > Signed-off-by: Christian Luciano Moreno > --- > drivers/staging/rtl8712/rtl8712_cmdctrl_bitdef.h | 1 - > 1 file changed, 1 deletion(-) I can't take patches without any changelog text :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8712: Fix Alignment CHECK
On Sat, Jul 13, 2019 at 06:47:20PM -0300, christianlucian...@gmail.com wrote:
> From: Christian Luciano Moreno
>
> Fix alignment check reported by checkpatch.
>
> Signed-off-by: Christian Luciano Moreno
> ---
> drivers/staging/rtl8712/recv_linux.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8712/recv_linux.c
> b/drivers/staging/rtl8712/recv_linux.c
> index 4e20cbafa9fb..97c980a039bd 100644
> --- a/drivers/staging/rtl8712/recv_linux.c
> +++ b/drivers/staging/rtl8712/recv_linux.c
> @@ -61,7 +61,7 @@ int r8712_os_recvbuf_resource_alloc(struct _adapter
> *padapter,
>
> /*free os related resource in struct recv_buf*/
> int r8712_os_recvbuf_resource_free(struct _adapter *padapter,
> - struct recv_buf *precvbuf)
> +struct recv_buf *precvbuf)
> {
> if (precvbuf->pskb)
> dev_kfree_skb_any(precvbuf->pskb);
> --
> 2.22.0
As my patch-bot said, you need to send all of these as a patch series,
properly numbered, so I have a chance to apply them in the correct
order.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8712: rtl871x_xmit.c: Fix some CHECK and WARNING
On Mon, Jul 15, 2019 at 02:17:04PM -0300, christianlucian...@gmail.com wrote: > From: Christian Luciano Moreno > > Fix: line over 80 characters > space unnecesary after cast > alignment match open parenthesis > comparison to NULL > Those warnings and check were reported by checkpatch. > > Signed-off-by: Christian Luciano Moreno > --- > drivers/staging/rtl8712/rtl871x_xmit.c | 100 - > 1 file changed, 50 insertions(+), 50 deletions(-) Hi, This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree. You are receiving this message because of the following common error(s) as indicated below: - Your patch did many different things all at once, making it difficult to review. All Linux kernel patches need to only do one thing at a time. If you need to do multiple things (such as clean up all coding style issues in a file/driver), do it in a sequence of patches, each one doing only one thing. This will make it easier to review the patches to ensure that they are correct, and to help alleviate any merge issues that larger patches can cause. If you wish to discuss this problem further, or you have questions about how to resolve this issue, please feel free to respond to this email and Greg will reply once he has dug out from the pending patches received from other developers. thanks, greg k-h's patch email bot ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8712: Fix CHECK reported by checkpatch
On Sat, Jul 13, 2019 at 09:20:05PM -0300, christianlucian...@gmail.com wrote: > From: Christian Luciano Moreno > > Change local variable name to avoid CamelCase. > Align code to the open parenthesis to fix alignment issues. That is two different things, right? So it should be 2 patches. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8712: Fix Alignment CHECK
On Sat, Jul 13, 2019 at 06:47:20PM -0300, christianlucian...@gmail.com wrote:
> From: Christian Luciano Moreno
>
> Fix alignment check reported by checkpatch.
>
> Signed-off-by: Christian Luciano Moreno
> ---
> drivers/staging/rtl8712/recv_linux.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8712/recv_linux.c
> b/drivers/staging/rtl8712/recv_linux.c
> index 4e20cbafa9fb..97c980a039bd 100644
> --- a/drivers/staging/rtl8712/recv_linux.c
> +++ b/drivers/staging/rtl8712/recv_linux.c
> @@ -61,7 +61,7 @@ int r8712_os_recvbuf_resource_alloc(struct _adapter
> *padapter,
>
> /*free os related resource in struct recv_buf*/
> int r8712_os_recvbuf_resource_free(struct _adapter *padapter,
> - struct recv_buf *precvbuf)
> +struct recv_buf *precvbuf)
> {
> if (precvbuf->pskb)
> dev_kfree_skb_any(precvbuf->pskb);
> --
> 2.22.0
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- You sent multiple patches, yet no indication of which ones should be
applied in which order. Greg could just guess, but if you are
receiving this email, he guessed wrong and the patches didn't apply.
Please read the section entitled "The canonical patch format" in the
kernel file, Documentation/SubmittingPatches for a description of how
to do this so that Greg has a chance to apply these correctly.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8712: Add parenthesis to Macro argument
On Sat, Jul 13, 2019 at 06:26:52PM -0300, christianlucian...@gmail.com wrote: > From: Christian Luciano Moreno > > Add parenthesis to Macro argument to avoid precedence issues. > > Signed-off-by: Christian Luciano Moreno > --- > drivers/staging/rtl8712/basic_types.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/rtl8712/basic_types.h > b/drivers/staging/rtl8712/basic_types.h > index 4ad7f35b1644..3e6d4ff45a75 100644 > --- a/drivers/staging/rtl8712/basic_types.h > +++ b/drivers/staging/rtl8712/basic_types.h > @@ -21,7 +21,7 @@ > > #define SIZE_T __kernel_size_t > #define sint signed int > -#define FIELD_OFFSET(s, field) ((addr_t)&((s *)(0))->field) > +#define FIELD_OFFSET(s, field) ((addr_t)&(((s) *)(0))->(field)) > This change makes no sense, and it breaks the build, which implies you did not test it at all :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723bs: core: Remove code valid only for 5GHz
On Mon, Jul 15, 2019 at 08:51:30AM +0200, Emanuel Bennici wrote: > As per TODO ,remove code valid only for 5 GHz(channel > 14). > > Signed-off-by: Hariprasad Kelam > Reviewed-by: Emanuel Bennici > --- > drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 6 +- > 1 file changed, 1 insertion(+), 5 deletions(-) Please do not send html email to a kernel mailing list :( Also, what did you do here? Why resend the whole thing? confused, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: kpc2000: Convert put_page() to put_user_page*()
There have been issues with get_user_pages and filesystem writeback.
The issues are better described in [1].
The solution being proposed wants to keep track of gup_pinned pages which will
allow to take furthur steps to coordinate between subsystems using gup.
put_user_page() simply calls put_page inside for now. But the implementation
will change once all call sites of put_page() are converted.
I currently do not have the driver to test. Could I have some suggestions to
test this code? The solution is currently implemented in [2] and
it would be great if we could apply the patch on top of [2] and run some tests
to check if any regressions occur.
[1] https://lwn.net/Articles/753027/
[2] https://github.com/johnhubbard/linux/tree/gup_dma_core
Cc: Matt Sickler
Cc: Greg Kroah-Hartman
Cc: Jérôme Glisse
Cc: Ira Weiny
Cc: John Hubbard
Cc: linux...@kvack.org
Cc: de...@driverdev.osuosl.org
Signed-off-by: Bharath Vedartham
---
drivers/staging/kpc2000/kpc_dma/fileops.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c
b/drivers/staging/kpc2000/kpc_dma/fileops.c
index 6166587..82c70e6 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -198,9 +198,7 @@ int kpc_dma_transfer(struct dev_private_data *priv, struct
kiocb *kcb, unsigned
sg_free_table(>sgt);
err_dma_map_sg:
err_alloc_sg_table:
- for (i = 0 ; i < acd->page_count ; i++){
- put_page(acd->user_pages[i]);
- }
+ put_user_pages(acd->user_pages, acd->page_count);
err_get_user_pages:
kfree(acd->user_pages);
err_alloc_userpages:
@@ -229,9 +227,7 @@ void transfer_complete_cb(struct aio_cb_data *acd, size_t
xfr_count, u32 flags)
dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents,
acd->ldev->dir);
- for (i = 0 ; i < acd->page_count ; i++){
- put_page(acd->user_pages[i]);
- }
+ put_user_pages(acd->user_pages, acd->page_count);
sg_free_table(>sgt);
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/7] staging: most: Use spinlock_t instead of struct spinlock
On Thu, Jul 04, 2019 at 05:38:00PM +0200, Sebastian Andrzej Siewior wrote:
> For spinlocks the type spinlock_t should be used instead of "struct
> spinlock".
>
> Use spinlock_t for spinlock's definition.
>
> Cc: Greg Kroah-Hartman
> Cc: Christian Gromm
> Cc: de...@driverdev.osuosl.org
> Signed-off-by: Sebastian Andrzej Siewior
> ---
> drivers/staging/most/net/net.c | 3 +--
> drivers/staging/most/video/video.c | 3 +--
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/most/net/net.c b/drivers/staging/most/net/net.c
> index c8a64e2090273..09b604df45e63 100644
> --- a/drivers/staging/most/net/net.c
> +++ b/drivers/staging/most/net/net.c
> @@ -69,7 +69,7 @@ struct net_dev_context {
>
> static struct list_head net_devices = LIST_HEAD_INIT(net_devices);
> static struct mutex probe_disc_mt; /* ch->linked = true, most_nd_open */
> -static struct spinlock list_lock; /* list_head, ch->linked = false, dev_hold
> */
> +static DEFINE_SPINLOCK(list_lock); /* list_head, ch->linked = false,
> dev_hold */
> static struct core_component comp;
>
> static int skb_to_mamac(const struct sk_buff *skb, struct mbo *mbo)
> @@ -507,7 +507,6 @@ static struct core_component comp = {
>
> static int __init most_net_init(void)
> {
> - spin_lock_init(_lock);
> mutex_init(_disc_mt);
> return most_register_component();
> }
> diff --git a/drivers/staging/most/video/video.c
> b/drivers/staging/most/video/video.c
> index adca250062e1b..fcd9e111e8bd0 100644
> --- a/drivers/staging/most/video/video.c
> +++ b/drivers/staging/most/video/video.c
> @@ -54,7 +54,7 @@ struct comp_fh {
> };
>
> static struct list_head video_devices = LIST_HEAD_INIT(video_devices);
> -static struct spinlock list_lock;
> +static DEFINE_SPINLOCK(list_lock);
>
> static inline bool data_ready(struct most_video_dev *mdev)
> {
> @@ -540,7 +540,6 @@ static struct core_component comp = {
>
> static int __init comp_init(void)
> {
> - spin_lock_init(_lock);
> return most_register_component();
> }
>
Does not apply on top of Linus's tree right now :(
Can you rebase and resend once 5.3-rc1 is out?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] mm/gup: Use put_user_page*() instead of put_page*()
On Mon, Jul 15, 2019 at 11:10:20AM -0700, John Hubbard wrote: > On 7/14/19 11:56 PM, Bharath Vedartham wrote: > > On Sun, Jul 14, 2019 at 04:33:42PM -0700, John Hubbard wrote: > >> On 7/14/19 12:08 PM, Bharath Vedartham wrote: > [...] > >> 1. Pull down https://github.com/johnhubbard/linux/commits/gup_dma_core > >> and find missing conversions: look for any additional missing > >> get_user_pages/put_page conversions. You've already found a couple missing > >> ones. I haven't re-run a search in a long time, so there's probably even > >> more. > >>a) And find more, after I rebase to 5.3-rc1: people probably are adding > >>get_user_pages() calls as we speak. :) > > Shouldn't this be documented then? I don't see any docs for using > > put_user_page*() in v5.2.1 in the memory management API section? > > Yes, it needs documentation. My first try (which is still in the above git > repo) was reviewed and found badly wanting, so I'm going to rewrite it. > Meanwhile, > I agree that an interim note would be helpful, let me put something together. > > [...] > >> https://github.com/johnhubbard/linux/commits/gup_dma_core > >> > >> a) gets rebased often, and > >> > >> b) has a bunch of commits (iov_iter and related) that conflict > >>with the latest linux.git, > >> > >> c) has some bugs in the bio area, that I'm fixing, so I don't trust > >>that's it's safely runnable, for a few more days. > > I assume your repo contains only work related to fixing gup issues and > > not the main repo for gup development? i.e where gup changes are merged? > > Correct, this is just a private tree, not a maintainer tree. But I'll try to > keep the gup_dma_core branch something that is usable by others, during the > transition over to put_user_page(), because the page-tracking patches are the > main way to test any put_user_page() conversions. > > As Ira said, we're using linux-mm as the real (maintainer) tree. Thanks for the info! > > thanks, > -- > John Hubbard > NVIDIA ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] mm/gup: Use put_user_page*() instead of put_page*()
On Mon, Jul 15, 2019 at 09:29:53AM -0700, Ira Weiny wrote: > On Mon, Jul 15, 2019 at 12:26:54PM +0530, Bharath Vedartham wrote: > > On Sun, Jul 14, 2019 at 04:33:42PM -0700, John Hubbard wrote: > > > On 7/14/19 12:08 PM, Bharath Vedartham wrote: > > > > This patch converts all call sites of get_user_pages > > > > to use put_user_page*() instead of put_page*() functions to > > > > release reference to gup pinned pages. > > Hi John, > > > Hi Bharath, > > > > > > Thanks for jumping in to help, and welcome to the party! > > > > > > You've caught everyone in the middle of a merge window, btw. As a > > > result, I'm busy rebasing and reworking the get_user_pages call sites, > > > and gup tracking, in the wake of some semi-traumatic changes to bio > > > and gup and such. I plan to re-post right after 5.3-rc1 shows up, from > > > here: > > > > > > https://github.com/johnhubbard/linux/commits/gup_dma_core > > > > > > ...which you'll find already covers the changes you've posted, except for: > > > > > > drivers/misc/sgi-gru/grufault.c > > > drivers/staging/kpc2000/kpc_dma/fileops.c > > > > > > ...and this one, which is undergoing to larger local changes, due to > > > bvec, so let's leave it out of the choices: > > > > > > fs/io_uring.c > > > > > > Therefore, until -rc1, if you'd like to help, I'd recommend one or more > > > of the following ideas: > > > > > > 1. Pull down https://github.com/johnhubbard/linux/commits/gup_dma_core > > > and find missing conversions: look for any additional missing > > > get_user_pages/put_page conversions. You've already found a couple > > > missing > > > ones. I haven't re-run a search in a long time, so there's probably even > > > more. > > > a) And find more, after I rebase to 5.3-rc1: people probably are adding > > > get_user_pages() calls as we speak. :) > > Shouldn't this be documented then? I don't see any docs for using > > put_user_page*() in v5.2.1 in the memory management API section? > > > 2. Patches: Focus on just one subsystem at a time, and perfect the patch > > > for > > > it. For example, I think this the staging driver would be perfect to > > > start with: > > > > > > drivers/staging/kpc2000/kpc_dma/fileops.c > > > > > > a) verify that you've really, corrected converted the whole > > > driver. (Hint: I think you might be overlooking a put_page call.) > > Yup. I did see that! Will fix it! > > > b) Attempt to test it if you can (I'm being hypocritical in > > > the extreme here, but one of my problems is that testing > > > has been light, so any help is very valuable). qemu...? > > > OTOH, maybe even qemu cannot easily test a kpc2000, but > > > perhaps `git blame` and talking to the authors would help > > > figure out a way to validate the changes. > > Great! I ll do that, I ll mail the patch authors and ask them for help > > in testing. > > > Thinking about whether you can run a test that would prove or > > > disprove my claim in (a), above, could be useful in coming up > > > with tests to run. > > > > > In other words, a few very high quality conversions (even just one) that > > > we can really put our faith in, is what I value most here. Tested patches > > > are awesome. > > I understand that! > > > 3. Once I re-post, turn on the new CONFIG_DEBUG_GET_USER_PAGES_REFERENCES > > > and run things such as xfstest/fstest. (Again, doing so would be going > > > further than I have yet--very helpful). Help clarify what conversions have > > > actually been tested and work, and which ones remain unvalidated. > > > Other: Please note that this: > > Yup will do that. > > > https://github.com/johnhubbard/linux/commits/gup_dma_core > > > > > > a) gets rebased often, and > > > > > > b) has a bunch of commits (iov_iter and related) that conflict > > >with the latest linux.git, > > > > > > c) has some bugs in the bio area, that I'm fixing, so I don't trust > > >that's it's safely runnable, for a few more days. > > I assume your repo contains only work related to fixing gup issues and > > not the main repo for gup development? i.e where gup changes are merged? > > We have been using Andrews tree for merging. > > > Also are release_pages and put_user_pages interchangable? > > Conceptually yes. But release_pages is more efficient. There was some > discussion around this starting here: > > https://lore.kernel.org/lkml/20190523172852.ga27...@iweiny-desk2.sc.intel.com/ > > And a resulting bug fix. > > https://lkml.org/lkml/2019/6/21/95 > > Ira Thanks! Will have a look at these links! > > > One note below, for the future: > > > > > > > > > > > This is a bunch of trivial conversions which is a part of an effort > > > > by John Hubbard to solve issues with gup pinned pages and > > > > filesystem writeback. > > > > > > > > The issue is more clearly described in John Hubbard's patch[1] where > > > > put_user_page*() functions are introduced. > > > > > > > > Currently
Re: [PATCH] mm/gup: Use put_user_page*() instead of put_page*()
On 7/14/19 11:56 PM, Bharath Vedartham wrote: > On Sun, Jul 14, 2019 at 04:33:42PM -0700, John Hubbard wrote: >> On 7/14/19 12:08 PM, Bharath Vedartham wrote: [...] >> 1. Pull down https://github.com/johnhubbard/linux/commits/gup_dma_core >> and find missing conversions: look for any additional missing >> get_user_pages/put_page conversions. You've already found a couple missing >> ones. I haven't re-run a search in a long time, so there's probably even >> more. >> a) And find more, after I rebase to 5.3-rc1: people probably are adding >> get_user_pages() calls as we speak. :) > Shouldn't this be documented then? I don't see any docs for using > put_user_page*() in v5.2.1 in the memory management API section? Yes, it needs documentation. My first try (which is still in the above git repo) was reviewed and found badly wanting, so I'm going to rewrite it. Meanwhile, I agree that an interim note would be helpful, let me put something together. [...] >> https://github.com/johnhubbard/linux/commits/gup_dma_core >> >> a) gets rebased often, and >> >> b) has a bunch of commits (iov_iter and related) that conflict >>with the latest linux.git, >> >> c) has some bugs in the bio area, that I'm fixing, so I don't trust >>that's it's safely runnable, for a few more days. > I assume your repo contains only work related to fixing gup issues and > not the main repo for gup development? i.e where gup changes are merged? Correct, this is just a private tree, not a maintainer tree. But I'll try to keep the gup_dma_core branch something that is usable by others, during the transition over to put_user_page(), because the page-tracking patches are the main way to test any put_user_page() conversions. As Ira said, we're using linux-mm as the real (maintainer) tree. thanks, -- John Hubbard NVIDIA ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8723bs: core: Change return type of init_mlme_ext_priv
As init_mlme_ext_priv function always returns SUCCESS , We can change
return type from int to void.
Fixes below issue identified by coccicheck
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:464:5-8: Unneeded
variable: "res". Return "_SUCCESS" on line 492
Signed-off-by: Hariprasad Kelam
---
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c| 6 +-
drivers/staging/rtl8723bs/include/rtw_mlme_ext.h | 2 +-
drivers/staging/rtl8723bs/os_dep/os_intfs.c | 6 +-
3 files changed, 3 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index 4285844..0629117 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -459,9 +459,8 @@ static u8 init_channel_set(struct adapter *padapter, u8
ChannelPlan, RT_CHANNEL_
return chanset_size;
}
-intinit_mlme_ext_priv(struct adapter *padapter)
+void init_mlme_ext_priv(struct adapter *padapter)
{
- int res = _SUCCESS;
struct registry_priv *pregistrypriv = >registrypriv;
struct mlme_ext_priv *pmlmeext = >mlmeextpriv;
struct mlme_priv *pmlmepriv = >mlmepriv;
@@ -488,9 +487,6 @@ int init_mlme_ext_priv(struct adapter *padapter)
#ifdef DBG_FIXED_CHAN
pmlmeext->fixed_chan = 0xFF;
#endif
-
- return res;
-
}
void free_mlme_ext_priv(struct mlme_ext_priv *pmlmeext)
diff --git a/drivers/staging/rtl8723bs/include/rtw_mlme_ext.h
b/drivers/staging/rtl8723bs/include/rtw_mlme_ext.h
index 733bb94..70cd8c0 100644
--- a/drivers/staging/rtl8723bs/include/rtw_mlme_ext.h
+++ b/drivers/staging/rtl8723bs/include/rtw_mlme_ext.h
@@ -535,7 +535,7 @@ struct mlme_ext_priv
};
void init_mlme_default_rate_set(struct adapter *padapter);
-int init_mlme_ext_priv(struct adapter *padapter);
+void init_mlme_ext_priv(struct adapter *padapter);
int init_hw_mlme_ext(struct adapter *padapter);
void free_mlme_ext_priv (struct mlme_ext_priv *pmlmeext);
extern void init_mlme_ext_timer(struct adapter *padapter);
diff --git a/drivers/staging/rtl8723bs/os_dep/os_intfs.c
b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
index 544e799..d2decb3 100644
--- a/drivers/staging/rtl8723bs/os_dep/os_intfs.c
+++ b/drivers/staging/rtl8723bs/os_dep/os_intfs.c
@@ -768,11 +768,7 @@ u8 rtw_init_drv_sw(struct adapter *padapter)
goto exit;
}
- if (init_mlme_ext_priv(padapter) == _FAIL) {
- RT_TRACE(_module_os_intfs_c_, _drv_err_, ("\n Can't init
mlme_ext_priv\n"));
- ret8 = _FAIL;
- goto exit;
- }
+ init_mlme_ext_priv(padapter);
if (_rtw_init_xmit_priv(>xmitpriv, padapter) == _FAIL) {
DBG_871X("Can't _rtw_init_xmit_priv\n");
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8712: rtl871x_xmit.c: Fix some CHECK and WARNING
From: Christian Luciano Moreno
Fix: line over 80 characters
space unnecesary after cast
alignment match open parenthesis
comparison to NULL
Those warnings and check were reported by checkpatch.
Signed-off-by: Christian Luciano Moreno
---
drivers/staging/rtl8712/rtl871x_xmit.c | 100 -
1 file changed, 50 insertions(+), 50 deletions(-)
diff --git a/drivers/staging/rtl8712/rtl871x_xmit.c
b/drivers/staging/rtl8712/rtl871x_xmit.c
index f6fe8ea12961..716a0eebdcbd 100644
--- a/drivers/staging/rtl8712/rtl871x_xmit.c
+++ b/drivers/staging/rtl8712/rtl871x_xmit.c
@@ -22,7 +22,6 @@
#include "osdep_intf.h"
#include "usb_ops.h"
-
static const u8 P802_1H_OUI[P80211_OUI_LEN] = {0x00, 0x00, 0xf8};
static const u8 RFC1042_OUI[P80211_OUI_LEN] = {0x00, 0x00, 0x00};
static void init_hwxmits(struct hw_xmit *phwxmit, sint entry);
@@ -39,7 +38,7 @@ static void _init_txservq(struct tx_servq *ptxservq)
void _r8712_init_sta_xmit_priv(struct sta_xmit_priv *psta_xmitpriv)
{
memset((unsigned char *)psta_xmitpriv, 0,
-sizeof(struct sta_xmit_priv));
+ sizeof(struct sta_xmit_priv));
spin_lock_init(_xmitpriv->lock);
_init_txservq(_xmitpriv->be_q);
_init_txservq(_xmitpriv->bk_q);
@@ -71,19 +70,21 @@ sint _r8712_init_xmit_priv(struct xmit_priv *pxmitpriv,
_init_queue(>apsd_queue);
_init_queue(>free_xmit_queue);
/*
-* Please allocate memory with the sz = (struct xmit_frame) *
NR_XMITFRAME,
+* Please allocate memory with the
+* sz = (struct xmit_frame) * NR_XMITFRAME,
* and initialize free_xmit_frame below.
* Please also apply free_txobj to link_up all the xmit_frames...
*/
pxmitpriv->pallocated_frame_buf =
- kmalloc(NR_XMITFRAME * sizeof(struct xmit_frame) + 4,
GFP_ATOMIC);
+ kmalloc(NR_XMITFRAME * sizeof(struct xmit_frame) + 4,
+ GFP_ATOMIC);
if (!pxmitpriv->pallocated_frame_buf) {
pxmitpriv->pxmit_frame_buf = NULL;
return _FAIL;
}
pxmitpriv->pxmit_frame_buf = pxmitpriv->pallocated_frame_buf + 4 -
- ((addr_t) (pxmitpriv->pallocated_frame_buf) & 3);
- pxframe = (struct xmit_frame *) pxmitpriv->pxmit_frame_buf;
+ ((addr_t)(pxmitpriv->pallocated_frame_buf) & 3);
+ pxframe = (struct xmit_frame *)pxmitpriv->pxmit_frame_buf;
for (i = 0; i < NR_XMITFRAME; i++) {
INIT_LIST_HEAD(&(pxframe->list));
pxframe->padapter = padapter;
@@ -92,7 +93,7 @@ sint _r8712_init_xmit_priv(struct xmit_priv *pxmitpriv,
pxframe->buf_addr = NULL;
pxframe->pxmitbuf = NULL;
list_add_tail(&(pxframe->list),
-&(pxmitpriv->free_xmit_queue.queue));
+ &(pxmitpriv->free_xmit_queue.queue));
pxframe++;
}
pxmitpriv->free_xmitframe_cnt = NR_XMITFRAME;
@@ -126,16 +127,16 @@ sint _r8712_init_xmit_priv(struct xmit_priv *pxmitpriv,
pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmitbuf;
for (i = 0; i < NR_XMITBUFF; i++) {
INIT_LIST_HEAD(>list);
- pxmitbuf->pallocated_buf = kmalloc(MAX_XMITBUF_SZ +
XMITBUF_ALIGN_SZ,
- GFP_ATOMIC);
+ pxmitbuf->pallocated_buf =
+ kmalloc(MAX_XMITBUF_SZ + XMITBUF_ALIGN_SZ, GFP_ATOMIC);
if (!pxmitbuf->pallocated_buf)
return _FAIL;
pxmitbuf->pbuf = pxmitbuf->pallocated_buf + XMITBUF_ALIGN_SZ -
-((addr_t) (pxmitbuf->pallocated_buf) &
+((addr_t)(pxmitbuf->pallocated_buf) &
(XMITBUF_ALIGN_SZ - 1));
r8712_xmit_resource_alloc(padapter, pxmitbuf);
list_add_tail(>list,
-&(pxmitpriv->free_xmitbuf_queue.queue));
+ &(pxmitpriv->free_xmitbuf_queue.queue));
pxmitbuf++;
}
pxmitpriv->free_xmitbuf_cnt = NR_XMITBUFF;
@@ -143,8 +144,8 @@ sint _r8712_init_xmit_priv(struct xmit_priv *pxmitpriv,
alloc_hwxmits(padapter);
init_hwxmits(pxmitpriv->hwxmits, pxmitpriv->hwxmit_entry);
tasklet_init(>xmit_tasklet,
- (void(*)(unsigned long))r8712_xmit_bh,
- (unsigned long)padapter);
+(void(*)(unsigned long))r8712_xmit_bh,
+(unsigned long)padapter);
return _SUCCESS;
}
@@ -156,7 +157,7 @@ void _free_xmit_priv(struct xmit_priv *pxmitpriv)
pxmitpriv->pxmit_frame_buf;
struct xmit_buf *pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmitbuf;
- if (pxmitpriv->pxmit_frame_buf == NULL)
+ if
Re: [PATCH] mm/gup: Use put_user_page*() instead of put_page*()
On Mon, Jul 15, 2019 at 12:26:54PM +0530, Bharath Vedartham wrote: > On Sun, Jul 14, 2019 at 04:33:42PM -0700, John Hubbard wrote: > > On 7/14/19 12:08 PM, Bharath Vedartham wrote: > > > This patch converts all call sites of get_user_pages > > > to use put_user_page*() instead of put_page*() functions to > > > release reference to gup pinned pages. > Hi John, > > Hi Bharath, > > > > Thanks for jumping in to help, and welcome to the party! > > > > You've caught everyone in the middle of a merge window, btw. As a > > result, I'm busy rebasing and reworking the get_user_pages call sites, > > and gup tracking, in the wake of some semi-traumatic changes to bio > > and gup and such. I plan to re-post right after 5.3-rc1 shows up, from > > here: > > > > https://github.com/johnhubbard/linux/commits/gup_dma_core > > > > ...which you'll find already covers the changes you've posted, except for: > > > > drivers/misc/sgi-gru/grufault.c > > drivers/staging/kpc2000/kpc_dma/fileops.c > > > > ...and this one, which is undergoing to larger local changes, due to > > bvec, so let's leave it out of the choices: > > > > fs/io_uring.c > > > > Therefore, until -rc1, if you'd like to help, I'd recommend one or more > > of the following ideas: > > > > 1. Pull down https://github.com/johnhubbard/linux/commits/gup_dma_core > > and find missing conversions: look for any additional missing > > get_user_pages/put_page conversions. You've already found a couple missing > > ones. I haven't re-run a search in a long time, so there's probably even > > more. > > a) And find more, after I rebase to 5.3-rc1: people probably are adding > > get_user_pages() calls as we speak. :) > Shouldn't this be documented then? I don't see any docs for using > put_user_page*() in v5.2.1 in the memory management API section? > > 2. Patches: Focus on just one subsystem at a time, and perfect the patch for > > it. For example, I think this the staging driver would be perfect to start > > with: > > > > drivers/staging/kpc2000/kpc_dma/fileops.c > > > > a) verify that you've really, corrected converted the whole > > driver. (Hint: I think you might be overlooking a put_page call.) > Yup. I did see that! Will fix it! > > b) Attempt to test it if you can (I'm being hypocritical in > > the extreme here, but one of my problems is that testing > > has been light, so any help is very valuable). qemu...? > > OTOH, maybe even qemu cannot easily test a kpc2000, but > > perhaps `git blame` and talking to the authors would help > > figure out a way to validate the changes. > Great! I ll do that, I ll mail the patch authors and ask them for help > in testing. > > Thinking about whether you can run a test that would prove or > > disprove my claim in (a), above, could be useful in coming up > > with tests to run. > > > In other words, a few very high quality conversions (even just one) that > > we can really put our faith in, is what I value most here. Tested patches > > are awesome. > I understand that! > > 3. Once I re-post, turn on the new CONFIG_DEBUG_GET_USER_PAGES_REFERENCES > > and run things such as xfstest/fstest. (Again, doing so would be going > > further than I have yet--very helpful). Help clarify what conversions have > > actually been tested and work, and which ones remain unvalidated. > > Other: Please note that this: > Yup will do that. > > https://github.com/johnhubbard/linux/commits/gup_dma_core > > > > a) gets rebased often, and > > > > b) has a bunch of commits (iov_iter and related) that conflict > >with the latest linux.git, > > > > c) has some bugs in the bio area, that I'm fixing, so I don't trust > >that's it's safely runnable, for a few more days. > I assume your repo contains only work related to fixing gup issues and > not the main repo for gup development? i.e where gup changes are merged? We have been using Andrews tree for merging. > Also are release_pages and put_user_pages interchangable? Conceptually yes. But release_pages is more efficient. There was some discussion around this starting here: https://lore.kernel.org/lkml/20190523172852.ga27...@iweiny-desk2.sc.intel.com/ And a resulting bug fix. https://lkml.org/lkml/2019/6/21/95 Ira > > One note below, for the future: > > > > > > > > This is a bunch of trivial conversions which is a part of an effort > > > by John Hubbard to solve issues with gup pinned pages and > > > filesystem writeback. > > > > > > The issue is more clearly described in John Hubbard's patch[1] where > > > put_user_page*() functions are introduced. > > > > > > Currently put_user_page*() simply does put_page but future implementations > > > look to change that once treewide change of put_page callsites to > > > put_user_page*() is finished. > > > > > > The lwn article describing the issue with gup pinned pages and filesystem > > > writeback [2]. > > > > > > This patch has
[PATCH] staging: rtl8712: rtl871x_xmit.h: Fix alignment code
From: Christian Luciano Moreno
Align code to match open parenthesis and tabs before statements.
Signed-off-by: Christian Luciano Moreno
---
drivers/staging/rtl8712/rtl871x_xmit.h | 30 +-
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/rtl8712/rtl871x_xmit.h
b/drivers/staging/rtl8712/rtl871x_xmit.h
index 3bea2e374f13..888c6bb24587 100644
--- a/drivers/staging/rtl8712/rtl871x_xmit.h
+++ b/drivers/staging/rtl8712/rtl871x_xmit.h
@@ -41,8 +41,8 @@ do { \
pattrib_iv[0] = txpn._byte_.TSC0;\
pattrib_iv[1] = txpn._byte_.TSC1;\
pattrib_iv[2] = txpn._byte_.TSC2;\
- pattrib_iv[3] = ((keyidx & 0x3)<<6);\
- txpn.val = (txpn.val == 0xff) ? 0 : (txpn.val+1);\
+ pattrib_iv[3] = ((keyidx & 0x3) << 6);\
+ txpn.val = (txpn.val == 0xff) ? 0 : (txpn.val + 1);\
} while (0)
/* Fixed the Big Endian bug when doing the Tx.
@@ -53,13 +53,13 @@ do { \
pattrib_iv[0] = txpn._byte_.TSC1;\
pattrib_iv[1] = (txpn._byte_.TSC1 | 0x20) & 0x7f;\
pattrib_iv[2] = txpn._byte_.TSC0;\
- pattrib_iv[3] = BIT(5) | ((keyidx & 0x3)<<6);\
+ pattrib_iv[3] = BIT(5) | ((keyidx & 0x3) << 6);\
pattrib_iv[4] = txpn._byte_.TSC2;\
pattrib_iv[5] = txpn._byte_.TSC3;\
pattrib_iv[6] = txpn._byte_.TSC4;\
pattrib_iv[7] = txpn._byte_.TSC5;\
txpn.val = txpn.val == 0xULL ? 0 : \
- (txpn.val+1);\
+ (txpn.val + 1);\
} while (0)
#define AES_IV(pattrib_iv, txpn, keyidx)\
@@ -67,13 +67,13 @@ do { \
pattrib_iv[0] = txpn._byte_.TSC0;\
pattrib_iv[1] = txpn._byte_.TSC1;\
pattrib_iv[2] = 0;\
- pattrib_iv[3] = BIT(5) | ((keyidx & 0x3)<<6);\
+ pattrib_iv[3] = BIT(5) | ((keyidx & 0x3) << 6);\
pattrib_iv[4] = txpn._byte_.TSC2;\
pattrib_iv[5] = txpn._byte_.TSC3;\
pattrib_iv[6] = txpn._byte_.TSC4;\
pattrib_iv[7] = txpn._byte_.TSC5;\
txpn.val = txpn.val == 0xULL ? 0 : \
- (txpn.val+1);\
+ (txpn.val + 1);\
} while (0)
struct hw_xmit {
@@ -148,8 +148,8 @@ struct xmit_frame {
_pkt *pkt;
int frame_tag;
struct _adapter *padapter;
-u8 *buf_addr;
-struct xmit_buf *pxmitbuf;
+ u8 *buf_addr;
+ struct xmit_buf *pxmitbuf;
u8 *mem_addr;
u16 sz[8];
struct urb *pxmit_urb[8];
@@ -182,11 +182,11 @@ struct sta_xmit_priv {
};
struct hw_txqueue {
- /*volatile*/ sint head;
- /*volatile*/ sint tail;
- /*volatile*/ sint free_sz;/*in units of 64 bytes*/
- /*volatile*/ sint free_cmdsz;
- /*volatile*/ sinttxsz[8];
+ sinthead; /*volatile*/
+ sinttail; /*volatile*/
+ sintfree_sz;/*volatile*/ /*in units of 64 bytes*/
+ sintfree_cmdsz; /*volatile*/
+ sinttxsz[8];/*volatile*/
uintff_hwaddr;
uintcmd_hwaddr;
sintac_tag;
@@ -259,7 +259,7 @@ void r8712_free_xmitframe(struct xmit_priv *pxmitpriv,
void r8712_free_xmitframe_queue(struct xmit_priv *pxmitpriv,
struct __queue *pframequeue);
sint r8712_xmit_classifier(struct _adapter *padapter,
- struct xmit_frame *pxmitframe);
+ struct xmit_frame *pxmitframe);
sint r8712_xmitframe_coalesce(struct _adapter *padapter, _pkt *pkt,
struct xmit_frame *pxmitframe);
sint _r8712_init_hw_txqueue(struct hw_txqueue *phw_txqueue, u8 ac_tag);
@@ -280,7 +280,7 @@ int r8712_xmit_direct(struct _adapter *padapter, struct
xmit_frame *pxmitframe);
void r8712_xmit_bh(void *priv);
void xmitframe_xmitbuf_attach(struct xmit_frame *pxmitframe,
- struct xmit_buf *pxmitbuf);
+ struct xmit_buf *pxmitbuf);
#include "rtl8712_xmit.h"
--
2.22.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: fbtft: Fix GPIO handling
On Tue, 2019-07-16 at 00:04 +0900, Jan Sebastian Götte wrote:
> Commit c440eee1a7a1 ("Staging: fbtft: Switch to the gpio descriptor
> interface") breaks GPIO handling. In several places, checks to only set
> a GPIO if it was configured ended up backwards.
> I have tested this fix. The fixed driver works with a ili9486
> display connected to a raspberry pi via SPI.
>
> Fixes: commit c440eee1a7a1d ("Staging: fbtft: Switch to the gpio descriptor
> interface")
> Tested-by: Jan Sebastian Götte
> Signed-off-by: Jan Sebastian Götte
Thanks Jan, this version is indeed more complete.
Reviewed-by: Nicolas Saenz Julienne
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: fbtft: Fix wrong check in,fbtft_write_wmem16_bus8()
Coincidentially, I've worked on the exact same issue this weekend. I can
confirm this change is necessary, and with this and the other two patches from
Phil Reid the driver works again. The same mistake occurred in several other
locations, though. I'll send a patch fixing all of them.
I've tested this on a ili9486-based display connected to a raspberry pi 3b+.
Regards, Jan
On Mon, 15 Jul 2019 Nicolas Saenz Julienne wrote:
> We actually want to set the gpio pin if it's avilable, not the other way
> around.
>
> Fixes: c440eee1a7a1 ("Staging: fbtft: Switch to the gpio descriptor
> interface")
> Signed-off-by: Nicolas Saenz Julienne
> ---
> drivers/staging/fbtft/fbtft-bus.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/fbtft/fbtft-bus.c
> b/drivers/staging/fbtft/fbtft-bus.c
> index 2ea814d0dca5..63c65dd67b17 100644
> --- a/drivers/staging/fbtft/fbtft-bus.c
> +++ b/drivers/staging/fbtft/fbtft-bus.c
> @@ -135,7 +135,7 @@ int fbtft_write_vmem16_bus8(struct fbtft_par *par,
> size_t offset, size_t len)
> remain = len / 2;
> vmem16 = (u16 *)(par->info->screen_buffer + offset);
> - if (!par->gpio.dc)
> + if (par->gpio.dc)
> gpiod_set_value(par->gpio.dc, 1);
> /* non buffered write */
> --
> 2.22.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: fbtft: Fix GPIO handling
Commit c440eee1a7a1 ("Staging: fbtft: Switch to the gpio descriptor
interface") breaks GPIO handling. In several places, checks to only set
a GPIO if it was configured ended up backwards.
I have tested this fix. The fixed driver works with a ili9486
display connected to a raspberry pi via SPI.
Fixes: commit c440eee1a7a1d ("Staging: fbtft: Switch to the gpio descriptor
interface")
Tested-by: Jan Sebastian Götte
Signed-off-by: Jan Sebastian Götte
---
drivers/staging/fbtft/fb_bd663474.c | 2 +-
drivers/staging/fbtft/fb_ili9163.c | 2 +-
drivers/staging/fbtft/fb_ili9325.c | 2 +-
drivers/staging/fbtft/fb_s6d1121.c | 2 +-
drivers/staging/fbtft/fb_ssd1289.c | 2 +-
drivers/staging/fbtft/fb_ssd1331.c | 4 ++--
drivers/staging/fbtft/fb_upd161704.c | 2 +-
drivers/staging/fbtft/fbtft-bus.c| 2 +-
drivers/staging/fbtft/fbtft-core.c | 4 ++--
9 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/fbtft/fb_bd663474.c
b/drivers/staging/fbtft/fb_bd663474.c
index b6c6d66e4eb1..e2c7646588f8 100644
--- a/drivers/staging/fbtft/fb_bd663474.c
+++ b/drivers/staging/fbtft/fb_bd663474.c
@@ -24,7 +24,7 @@
static int init_display(struct fbtft_par *par)
{
- if (!par->gpio.cs)
+ if (par->gpio.cs)
gpiod_set_value(par->gpio.cs, 0); /* Activate chip */
par->fbtftops.reset(par);
diff --git a/drivers/staging/fbtft/fb_ili9163.c
b/drivers/staging/fbtft/fb_ili9163.c
index d609a2b67db9..fd32376700e2 100644
--- a/drivers/staging/fbtft/fb_ili9163.c
+++ b/drivers/staging/fbtft/fb_ili9163.c
@@ -77,7 +77,7 @@ static int init_display(struct fbtft_par *par)
{
par->fbtftops.reset(par);
- if (!par->gpio.cs)
+ if (par->gpio.cs)
gpiod_set_value(par->gpio.cs, 0); /* Activate chip */
write_reg(par, MIPI_DCS_SOFT_RESET); /* software reset */
diff --git a/drivers/staging/fbtft/fb_ili9325.c
b/drivers/staging/fbtft/fb_ili9325.c
index b090e7ab6fdd..85e54a10ed72 100644
--- a/drivers/staging/fbtft/fb_ili9325.c
+++ b/drivers/staging/fbtft/fb_ili9325.c
@@ -85,7 +85,7 @@ static int init_display(struct fbtft_par *par)
{
par->fbtftops.reset(par);
- if (!par->gpio.cs)
+ if (par->gpio.cs)
gpiod_set_value(par->gpio.cs, 0); /* Activate chip */
bt &= 0x07;
diff --git a/drivers/staging/fbtft/fb_s6d1121.c
b/drivers/staging/fbtft/fb_s6d1121.c
index b3d0701880fe..5a129b1352cc 100644
--- a/drivers/staging/fbtft/fb_s6d1121.c
+++ b/drivers/staging/fbtft/fb_s6d1121.c
@@ -29,7 +29,7 @@ static int init_display(struct fbtft_par *par)
{
par->fbtftops.reset(par);
- if (!par->gpio.cs)
+ if (par->gpio.cs)
gpiod_set_value(par->gpio.cs, 0); /* Activate chip */
/* Initialization sequence from Lib_UTFT */
diff --git a/drivers/staging/fbtft/fb_ssd1289.c
b/drivers/staging/fbtft/fb_ssd1289.c
index bbf75f795234..88a5b6925901 100644
--- a/drivers/staging/fbtft/fb_ssd1289.c
+++ b/drivers/staging/fbtft/fb_ssd1289.c
@@ -28,7 +28,7 @@ static int init_display(struct fbtft_par *par)
{
par->fbtftops.reset(par);
- if (!par->gpio.cs)
+ if (par->gpio.cs)
gpiod_set_value(par->gpio.cs, 0); /* Activate chip */
write_reg(par, 0x00, 0x0001);
diff --git a/drivers/staging/fbtft/fb_ssd1331.c
b/drivers/staging/fbtft/fb_ssd1331.c
index 4cfe9f8535d0..37622c9462aa 100644
--- a/drivers/staging/fbtft/fb_ssd1331.c
+++ b/drivers/staging/fbtft/fb_ssd1331.c
@@ -81,7 +81,7 @@ static void write_reg8_bus8(struct fbtft_par *par, int len,
...)
va_start(args, len);
*buf = (u8)va_arg(args, unsigned int);
- if (!par->gpio.dc)
+ if (par->gpio.dc)
gpiod_set_value(par->gpio.dc, 0);
ret = par->fbtftops.write(par, par->buf, sizeof(u8));
if (ret < 0) {
@@ -104,7 +104,7 @@ static void write_reg8_bus8(struct fbtft_par *par, int len,
...)
return;
}
}
- if (!par->gpio.dc)
+ if (par->gpio.dc)
gpiod_set_value(par->gpio.dc, 1);
va_end(args);
}
diff --git a/drivers/staging/fbtft/fb_upd161704.c
b/drivers/staging/fbtft/fb_upd161704.c
index 564a38e34440..c77832ae5e5b 100644
--- a/drivers/staging/fbtft/fb_upd161704.c
+++ b/drivers/staging/fbtft/fb_upd161704.c
@@ -26,7 +26,7 @@ static int init_display(struct fbtft_par *par)
{
par->fbtftops.reset(par);
- if (!par->gpio.cs)
+ if (par->gpio.cs)
gpiod_set_value(par->gpio.cs, 0); /* Activate chip */
/* Initialization sequence from Lib_UTFT */
diff --git a/drivers/staging/fbtft/fbtft-bus.c
b/drivers/staging/fbtft/fbtft-bus.c
index 2ea814d0dca5..63c65dd67b17 100644
--- a/drivers/staging/fbtft/fbtft-bus.c
+++ b/drivers/staging/fbtft/fbtft-bus.c
@@ -135,7 +135,7 @@ int fbtft_write_vmem16_bus8(struct fbtft_par *par, size_t
offset, size_t len)
remain = len / 2;
vmem16 = (u16 *)(par->info->screen_buffer
[PATCH AUTOSEL 4.4 16/53] media: staging: media: davinci_vpfe: - Fix for memory leak if decoder initialization fails.
From: Shailendra Verma
[ Upstream commit 6995a659101bd4effa41cebb067f9dc18d77520d ]
Fix to avoid possible memory leak if the decoder initialization
got failed.Free the allocated memory for file handle object
before return in case decoder initialization fails.
Signed-off-by: Shailendra Verma
Signed-off-by: Mauro Carvalho Chehab
Signed-off-by: Sasha Levin
---
drivers/staging/media/davinci_vpfe/vpfe_video.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c
b/drivers/staging/media/davinci_vpfe/vpfe_video.c
index 0fdff91624fd..43474f562b43 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
+++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
@@ -406,6 +406,9 @@ static int vpfe_open(struct file *file)
/* If decoder is not initialized. initialize it */
if (!video->initialized && vpfe_update_pipe_state(video)) {
mutex_unlock(>lock);
+ v4l2_fh_del(>vfh);
+ v4l2_fh_exit(>vfh);
+ kfree(handle);
return -ENODEV;
}
/* Increment device users counter */
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH AUTOSEL 4.9 20/73] media: staging: media: davinci_vpfe: - Fix for memory leak if decoder initialization fails.
From: Shailendra Verma
[ Upstream commit 6995a659101bd4effa41cebb067f9dc18d77520d ]
Fix to avoid possible memory leak if the decoder initialization
got failed.Free the allocated memory for file handle object
before return in case decoder initialization fails.
Signed-off-by: Shailendra Verma
Signed-off-by: Mauro Carvalho Chehab
Signed-off-by: Sasha Levin
---
drivers/staging/media/davinci_vpfe/vpfe_video.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c
b/drivers/staging/media/davinci_vpfe/vpfe_video.c
index 89dd6b989254..e0440807b4ed 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
+++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
@@ -423,6 +423,9 @@ static int vpfe_open(struct file *file)
/* If decoder is not initialized. initialize it */
if (!video->initialized && vpfe_update_pipe_state(video)) {
mutex_unlock(>lock);
+ v4l2_fh_del(>vfh);
+ v4l2_fh_exit(>vfh);
+ kfree(handle);
return -ENODEV;
}
/* Increment device users counter */
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH AUTOSEL 4.14 025/105] media: staging: media: davinci_vpfe: - Fix for memory leak if decoder initialization fails.
From: Shailendra Verma
[ Upstream commit 6995a659101bd4effa41cebb067f9dc18d77520d ]
Fix to avoid possible memory leak if the decoder initialization
got failed.Free the allocated memory for file handle object
before return in case decoder initialization fails.
Signed-off-by: Shailendra Verma
Signed-off-by: Mauro Carvalho Chehab
Signed-off-by: Sasha Levin
---
drivers/staging/media/davinci_vpfe/vpfe_video.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c
b/drivers/staging/media/davinci_vpfe/vpfe_video.c
index 155e8c758e4b..346e60d230f3 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
+++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
@@ -422,6 +422,9 @@ static int vpfe_open(struct file *file)
/* If decoder is not initialized. initialize it */
if (!video->initialized && vpfe_update_pipe_state(video)) {
mutex_unlock(>lock);
+ v4l2_fh_del(>vfh);
+ v4l2_fh_exit(>vfh);
+ kfree(handle);
return -ENODEV;
}
/* Increment device users counter */
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: fbtft: Fix wrong check in fbtft_write_wmem16_bus8()
We actually want to set the gpio pin if it's avilable, not the other way
around.
Fixes: c440eee1a7a1 ("Staging: fbtft: Switch to the gpio descriptor interface")
Signed-off-by: Nicolas Saenz Julienne
---
drivers/staging/fbtft/fbtft-bus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/fbtft/fbtft-bus.c
b/drivers/staging/fbtft/fbtft-bus.c
index 2ea814d0dca5..63c65dd67b17 100644
--- a/drivers/staging/fbtft/fbtft-bus.c
+++ b/drivers/staging/fbtft/fbtft-bus.c
@@ -135,7 +135,7 @@ int fbtft_write_vmem16_bus8(struct fbtft_par *par, size_t
offset, size_t len)
remain = len / 2;
vmem16 = (u16 *)(par->info->screen_buffer + offset);
- if (!par->gpio.dc)
+ if (par->gpio.dc)
gpiod_set_value(par->gpio.dc, 1);
/* non buffered write */
--
2.22.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/2] Staging: fbtft: Fix probing of gpio descriptor
On Thu, 2019-07-11 at 16:31 +0800, Phil Reid wrote:
> GPIO probing and reset polarity are broken.
> Fix them.
>
> Fixes: c440eee1a7a1 ("Staging: fbtft: Switch to the gpio descriptor
> interface")
>
> Phil Reid (2):
> Staging: fbtft: Fix probing of gpio descriptor
> Staging: fbtft: Fix reset assertion when using gpio descriptor
>
> drivers/staging/fbtft/fbtft-core.c | 43 ++---
> -
> 1 file changed, 20 insertions(+), 23 deletions(-)
>
You can add my:
Reviewed-by: Nicolas Saenz Julienne
Tested-by: Nicolas Saenz Julienne
The only issue I see is in the second patch, who should also have the same
'Fixes' tag.
BTW, while testing I found another issue, I'll send a fix shortly.
Kind regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH AUTOSEL 4.19 031/158] media: staging: media: davinci_vpfe: - Fix for memory leak if decoder initialization fails.
From: Shailendra Verma
[ Upstream commit 6995a659101bd4effa41cebb067f9dc18d77520d ]
Fix to avoid possible memory leak if the decoder initialization
got failed.Free the allocated memory for file handle object
before return in case decoder initialization fails.
Signed-off-by: Shailendra Verma
Signed-off-by: Mauro Carvalho Chehab
Signed-off-by: Sasha Levin
---
drivers/staging/media/davinci_vpfe/vpfe_video.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c
b/drivers/staging/media/davinci_vpfe/vpfe_video.c
index 1269a983455e..13b890b9ef18 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
+++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
@@ -422,6 +422,9 @@ static int vpfe_open(struct file *file)
/* If decoder is not initialized. initialize it */
if (!video->initialized && vpfe_update_pipe_state(video)) {
mutex_unlock(>lock);
+ v4l2_fh_del(>vfh);
+ v4l2_fh_exit(>vfh);
+ kfree(handle);
return -ENODEV;
}
/* Increment device users counter */
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH AUTOSEL 5.1 059/219] media: imx7-mipi-csis: Propagate the error if clock enabling fails
From: Fabio Estevam
[ Upstream commit 2b393f91c651c16d5c09f5c7aa689e58a79df34e ]
Currently the return value from clk_bulk_prepare_enable() is checked,
but it is not propagate it in the case of failure.
Fix it and also move the error message to the caller of
mipi_csis_clk_enable().
Signed-off-by: Fabio Estevam
Reviewed-by: Rui Miguel Silva
Signed-off-by: Hans Verkuil
Signed-off-by: Mauro Carvalho Chehab
Signed-off-by: Sasha Levin
---
drivers/staging/media/imx/imx7-mipi-csis.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c
b/drivers/staging/media/imx/imx7-mipi-csis.c
index 2ddcc42ab8ff..e9d621e19d6d 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -455,13 +455,9 @@ static void mipi_csis_set_params(struct csi_state *state)
MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL);
}
-static void mipi_csis_clk_enable(struct csi_state *state)
+static int mipi_csis_clk_enable(struct csi_state *state)
{
- int ret;
-
- ret = clk_bulk_prepare_enable(state->num_clks, state->clks);
- if (ret < 0)
- dev_err(state->dev, "failed to enable clocks\n");
+ return clk_bulk_prepare_enable(state->num_clks, state->clks);
}
static void mipi_csis_clk_disable(struct csi_state *state)
@@ -985,7 +981,11 @@ static int mipi_csis_probe(struct platform_device *pdev)
if (ret < 0)
return ret;
- mipi_csis_clk_enable(state);
+ ret = mipi_csis_clk_enable(state);
+ if (ret < 0) {
+ dev_err(state->dev, "failed to enable clocks: %d\n", ret);
+ return ret;
+ }
ret = devm_request_irq(dev, state->irq, mipi_csis_irq_handler,
0, dev_name(dev), state);
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH AUTOSEL 5.1 046/219] media: staging: media: davinci_vpfe: - Fix for memory leak if decoder initialization fails.
From: Shailendra Verma
[ Upstream commit 6995a659101bd4effa41cebb067f9dc18d77520d ]
Fix to avoid possible memory leak if the decoder initialization
got failed.Free the allocated memory for file handle object
before return in case decoder initialization fails.
Signed-off-by: Shailendra Verma
Signed-off-by: Mauro Carvalho Chehab
Signed-off-by: Sasha Levin
---
drivers/staging/media/davinci_vpfe/vpfe_video.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c
b/drivers/staging/media/davinci_vpfe/vpfe_video.c
index 510202a3b091..84cca18e3e9d 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
+++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
@@ -419,6 +419,9 @@ static int vpfe_open(struct file *file)
/* If decoder is not initialized. initialize it */
if (!video->initialized && vpfe_update_pipe_state(video)) {
mutex_unlock(>lock);
+ v4l2_fh_del(>vfh);
+ v4l2_fh_exit(>vfh);
+ kfree(handle);
return -ENODEV;
}
/* Increment device users counter */
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH AUTOSEL 5.2 142/249] media: staging: davinci: fix memory leaks and check for allocation failure
From: Colin Ian King
[ Upstream commit a84e355ecd3ed9759d7aaa40170aab78e2a68a06 ]
There are three error return paths that don't kfree params causing a
memory leak. Fix this by adding an error return path that kfree's
params before returning. Also add a check to see params failed to
be allocated.
Signed-off-by: Colin Ian King
Signed-off-by: Hans Verkuil
Signed-off-by: Mauro Carvalho Chehab
Signed-off-by: Sasha Levin
---
drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
index 30e2edc0cec5..b88855c7ffe8 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
@@ -1251,10 +1251,10 @@ static int ipipe_s_config(struct v4l2_subdev *sd,
struct vpfe_ipipe_config *cfg)
struct vpfe_ipipe_device *ipipe = v4l2_get_subdevdata(sd);
unsigned int i;
int rval = 0;
+ struct ipipe_module_params *params;
for (i = 0; i < ARRAY_SIZE(ipipe_modules); i++) {
const struct ipipe_module_if *module_if;
- struct ipipe_module_params *params;
void *from, *to;
size_t size;
@@ -1265,25 +1265,30 @@ static int ipipe_s_config(struct v4l2_subdev *sd,
struct vpfe_ipipe_config *cfg)
from = *(void **)((void *)cfg + module_if->config_offset);
params = kmalloc(sizeof(*params), GFP_KERNEL);
+ if (!params)
+ return -ENOMEM;
to = (void *)params + module_if->param_offset;
size = module_if->param_size;
if (to && from && size) {
if (copy_from_user(to, (void __user *)from, size)) {
rval = -EFAULT;
- break;
+ goto error_free;
}
rval = module_if->set(ipipe, to);
if (rval)
- goto error;
+ goto error_free;
} else if (to && !from && size) {
rval = module_if->set(ipipe, NULL);
if (rval)
- goto error;
+ goto error_free;
}
kfree(params);
}
-error:
+ return rval;
+
+error_free:
+ kfree(params);
return rval;
}
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH AUTOSEL 5.2 068/249] media: imx7-mipi-csis: Propagate the error if clock enabling fails
From: Fabio Estevam
[ Upstream commit 2b393f91c651c16d5c09f5c7aa689e58a79df34e ]
Currently the return value from clk_bulk_prepare_enable() is checked,
but it is not propagate it in the case of failure.
Fix it and also move the error message to the caller of
mipi_csis_clk_enable().
Signed-off-by: Fabio Estevam
Reviewed-by: Rui Miguel Silva
Signed-off-by: Hans Verkuil
Signed-off-by: Mauro Carvalho Chehab
Signed-off-by: Sasha Levin
---
drivers/staging/media/imx/imx7-mipi-csis.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c
b/drivers/staging/media/imx/imx7-mipi-csis.c
index 19455f425416..7d7bdfdd852a 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -456,13 +456,9 @@ static void mipi_csis_set_params(struct csi_state *state)
MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRL);
}
-static void mipi_csis_clk_enable(struct csi_state *state)
+static int mipi_csis_clk_enable(struct csi_state *state)
{
- int ret;
-
- ret = clk_bulk_prepare_enable(state->num_clks, state->clks);
- if (ret < 0)
- dev_err(state->dev, "failed to enable clocks\n");
+ return clk_bulk_prepare_enable(state->num_clks, state->clks);
}
static void mipi_csis_clk_disable(struct csi_state *state)
@@ -973,7 +969,11 @@ static int mipi_csis_probe(struct platform_device *pdev)
if (ret < 0)
return ret;
- mipi_csis_clk_enable(state);
+ ret = mipi_csis_clk_enable(state);
+ if (ret < 0) {
+ dev_err(state->dev, "failed to enable clocks: %d\n", ret);
+ return ret;
+ }
ret = devm_request_irq(dev, state->irq, mipi_csis_irq_handler,
0, dev_name(dev), state);
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH AUTOSEL 5.2 053/249] media: staging: media: davinci_vpfe: - Fix for memory leak if decoder initialization fails.
From: Shailendra Verma
[ Upstream commit 6995a659101bd4effa41cebb067f9dc18d77520d ]
Fix to avoid possible memory leak if the decoder initialization
got failed.Free the allocated memory for file handle object
before return in case decoder initialization fails.
Signed-off-by: Shailendra Verma
Signed-off-by: Mauro Carvalho Chehab
Signed-off-by: Sasha Levin
---
drivers/staging/media/davinci_vpfe/vpfe_video.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c
b/drivers/staging/media/davinci_vpfe/vpfe_video.c
index 510202a3b091..84cca18e3e9d 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
+++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
@@ -419,6 +419,9 @@ static int vpfe_open(struct file *file)
/* If decoder is not initialized. initialize it */
if (!video->initialized && vpfe_update_pipe_state(video)) {
mutex_unlock(>lock);
+ v4l2_fh_del(>vfh);
+ v4l2_fh_exit(>vfh);
+ kfree(handle);
return -ENODEV;
}
/* Increment device users counter */
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4] staging: erofs:converting all 'unsigned' to 'unsigned int'
Fixed checkpatch warnings: converting all 'unsigned' to 'unsigned int'
Signed-off-by: Pratik Shinde
Reviewed-by: Gao Xiang
---
drivers/staging/erofs/internal.h | 7 ---
drivers/staging/erofs/unzip_pagevec.h | 11 ++-
drivers/staging/erofs/unzip_vle.h | 8
drivers/staging/erofs/xattr.h | 17 +
4 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 963cc1b..0ebc294 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -359,8 +359,8 @@ struct erofs_vnode {
unsigned char inode_isize;
unsigned short xattr_isize;
- unsigned xattr_shared_count;
- unsigned *xattr_shared_xattrs;
+ unsigned int xattr_shared_count;
+ unsigned int *xattr_shared_xattrs;
union {
erofs_blk_t raw_blkaddr;
@@ -510,7 +510,8 @@ erofs_grab_bio(struct super_block *sb,
return bio;
}
-static inline void __submit_bio(struct bio *bio, unsigned op, unsigned
op_flags)
+static inline void __submit_bio(struct bio *bio, unsigned int op,
+ unsigned int op_flags)
{
bio_set_op_attrs(bio, op, op_flags);
submit_bio(bio);
diff --git a/drivers/staging/erofs/unzip_pagevec.h
b/drivers/staging/erofs/unzip_pagevec.h
index 7af0ba8..e65dbca 100644
--- a/drivers/staging/erofs/unzip_pagevec.h
+++ b/drivers/staging/erofs/unzip_pagevec.h
@@ -54,9 +54,9 @@ static inline void z_erofs_pagevec_ctor_exit(struct
z_erofs_pagevec_ctor *ctor,
static inline struct page *
z_erofs_pagevec_ctor_next_page(struct z_erofs_pagevec_ctor *ctor,
- unsigned nr)
+ unsigned int nr)
{
- unsigned index;
+ unsigned int index;
/* keep away from occupied pages */
if (ctor->next)
@@ -64,7 +64,7 @@ z_erofs_pagevec_ctor_next_page(struct z_erofs_pagevec_ctor
*ctor,
for (index = 0; index < nr; ++index) {
const erofs_vtptr_t t = ctor->pages[index];
- const unsigned tags = tagptr_unfold_tags(t);
+ const unsigned int tags = tagptr_unfold_tags(t);
if (tags == Z_EROFS_PAGE_TYPE_EXCLUSIVE)
return tagptr_unfold_ptr(t);
@@ -91,8 +91,9 @@ z_erofs_pagevec_ctor_pagedown(struct z_erofs_pagevec_ctor
*ctor,
}
static inline void z_erofs_pagevec_ctor_init(struct z_erofs_pagevec_ctor *ctor,
-unsigned nr,
-erofs_vtptr_t *pages, unsigned i)
+unsigned int nr,
+erofs_vtptr_t *pages,
+unsigned int i)
{
ctor->nr = nr;
ctor->curr = ctor->next = NULL;
diff --git a/drivers/staging/erofs/unzip_vle.h
b/drivers/staging/erofs/unzip_vle.h
index ab509d75..df91ad1 100644
--- a/drivers/staging/erofs/unzip_vle.h
+++ b/drivers/staging/erofs/unzip_vle.h
@@ -34,7 +34,7 @@ struct z_erofs_vle_work {
unsigned short nr_pages;
/* L: queued pages in pagevec[] */
- unsigned vcnt;
+ unsigned int vcnt;
union {
/* L: pagevec */
@@ -124,7 +124,7 @@ union z_erofs_onlinepage_converter {
unsigned long *v;
};
-static inline unsigned z_erofs_onlinepage_index(struct page *page)
+static inline unsigned int z_erofs_onlinepage_index(struct page *page)
{
union z_erofs_onlinepage_converter u;
@@ -164,7 +164,7 @@ static inline void z_erofs_onlinepage_fixup(struct page
*page,
}
v = (index << Z_EROFS_ONLINEPAGE_INDEX_SHIFT) |
- ((o & Z_EROFS_ONLINEPAGE_COUNT_MASK) + (unsigned)down);
+ ((o & Z_EROFS_ONLINEPAGE_COUNT_MASK) + (unsigned int)down);
if (cmpxchg(p, o, v) != o)
goto repeat;
}
@@ -172,7 +172,7 @@ static inline void z_erofs_onlinepage_fixup(struct page
*page,
static inline void z_erofs_onlinepage_endio(struct page *page)
{
union z_erofs_onlinepage_converter u;
- unsigned v;
+ unsigned int v;
DBG_BUGON(!PagePrivate(page));
u.v = _private(page);
diff --git a/drivers/staging/erofs/xattr.h b/drivers/staging/erofs/xattr.h
index 35ba5ac..3990805 100644
--- a/drivers/staging/erofs/xattr.h
+++ b/drivers/staging/erofs/xattr.h
@@ -20,14 +20,14 @@
/* Attribute not found */
#define ENOATTR ENODATA
-static inline unsigned inlinexattr_header_size(struct inode *inode)
+static inline unsigned int inlinexattr_header_size(struct inode *inode)
{
return sizeof(struct erofs_xattr_ibody_header)
+ sizeof(u32) * EROFS_V(inode)->xattr_shared_count;
}
-static inline erofs_blk_t
-xattrblock_addr(struct erofs_sb_info *sbi, unsigned xattr_id)
+static inline erofs_blk_t xattrblock_addr(struct erofs_sb_info *sbi,
+
Re: [PATCH] staging: erofs:converting all 'unsigned' to 'unsigned int'
On 2019/7/15 19:29, Pratik Shinde wrote:
> Fixed checkpatch warnings: converting all 'unsigned' to 'unsigned int'
>
> Signed-off-by: Pratik Shinde
Bump version number and add my reviewed-by tag if you resend again...
since no idea which version is the latest version...
Thanks,
Gao Xiang
> ---
> drivers/staging/erofs/internal.h | 7 ---
> drivers/staging/erofs/unzip_pagevec.h | 11 ++-
> drivers/staging/erofs/unzip_vle.h | 8
> drivers/staging/erofs/xattr.h | 17 +
> 4 files changed, 23 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/staging/erofs/internal.h
> b/drivers/staging/erofs/internal.h
> index 963cc1b..0ebc294 100644
> --- a/drivers/staging/erofs/internal.h
> +++ b/drivers/staging/erofs/internal.h
> @@ -359,8 +359,8 @@ struct erofs_vnode {
> unsigned char inode_isize;
> unsigned short xattr_isize;
>
> - unsigned xattr_shared_count;
> - unsigned *xattr_shared_xattrs;
> + unsigned int xattr_shared_count;
> + unsigned int *xattr_shared_xattrs;
>
> union {
> erofs_blk_t raw_blkaddr;
> @@ -510,7 +510,8 @@ erofs_grab_bio(struct super_block *sb,
> return bio;
> }
>
> -static inline void __submit_bio(struct bio *bio, unsigned op, unsigned
> op_flags)
> +static inline void __submit_bio(struct bio *bio, unsigned int op,
> + unsigned int op_flags)
> {
> bio_set_op_attrs(bio, op, op_flags);
> submit_bio(bio);
> diff --git a/drivers/staging/erofs/unzip_pagevec.h
> b/drivers/staging/erofs/unzip_pagevec.h
> index 7af0ba8..e65dbca 100644
> --- a/drivers/staging/erofs/unzip_pagevec.h
> +++ b/drivers/staging/erofs/unzip_pagevec.h
> @@ -54,9 +54,9 @@ static inline void z_erofs_pagevec_ctor_exit(struct
> z_erofs_pagevec_ctor *ctor,
>
> static inline struct page *
> z_erofs_pagevec_ctor_next_page(struct z_erofs_pagevec_ctor *ctor,
> -unsigned nr)
> +unsigned int nr)
> {
> - unsigned index;
> + unsigned int index;
>
> /* keep away from occupied pages */
> if (ctor->next)
> @@ -64,7 +64,7 @@ z_erofs_pagevec_ctor_next_page(struct z_erofs_pagevec_ctor
> *ctor,
>
> for (index = 0; index < nr; ++index) {
> const erofs_vtptr_t t = ctor->pages[index];
> - const unsigned tags = tagptr_unfold_tags(t);
> + const unsigned int tags = tagptr_unfold_tags(t);
>
> if (tags == Z_EROFS_PAGE_TYPE_EXCLUSIVE)
> return tagptr_unfold_ptr(t);
> @@ -91,8 +91,9 @@ z_erofs_pagevec_ctor_pagedown(struct z_erofs_pagevec_ctor
> *ctor,
> }
>
> static inline void z_erofs_pagevec_ctor_init(struct z_erofs_pagevec_ctor
> *ctor,
> - unsigned nr,
> - erofs_vtptr_t *pages, unsigned i)
> + unsigned int nr,
> + erofs_vtptr_t *pages,
> + unsigned int i)
> {
> ctor->nr = nr;
> ctor->curr = ctor->next = NULL;
> diff --git a/drivers/staging/erofs/unzip_vle.h
> b/drivers/staging/erofs/unzip_vle.h
> index ab509d75..df91ad1 100644
> --- a/drivers/staging/erofs/unzip_vle.h
> +++ b/drivers/staging/erofs/unzip_vle.h
> @@ -34,7 +34,7 @@ struct z_erofs_vle_work {
> unsigned short nr_pages;
>
> /* L: queued pages in pagevec[] */
> - unsigned vcnt;
> + unsigned int vcnt;
>
> union {
> /* L: pagevec */
> @@ -124,7 +124,7 @@ union z_erofs_onlinepage_converter {
> unsigned long *v;
> };
>
> -static inline unsigned z_erofs_onlinepage_index(struct page *page)
> +static inline unsigned int z_erofs_onlinepage_index(struct page *page)
> {
> union z_erofs_onlinepage_converter u;
>
> @@ -164,7 +164,7 @@ static inline void z_erofs_onlinepage_fixup(struct page
> *page,
> }
>
> v = (index << Z_EROFS_ONLINEPAGE_INDEX_SHIFT) |
> - ((o & Z_EROFS_ONLINEPAGE_COUNT_MASK) + (unsigned)down);
> + ((o & Z_EROFS_ONLINEPAGE_COUNT_MASK) + (unsigned int)down);
> if (cmpxchg(p, o, v) != o)
> goto repeat;
> }
> @@ -172,7 +172,7 @@ static inline void z_erofs_onlinepage_fixup(struct page
> *page,
> static inline void z_erofs_onlinepage_endio(struct page *page)
> {
> union z_erofs_onlinepage_converter u;
> - unsigned v;
> + unsigned int v;
>
> DBG_BUGON(!PagePrivate(page));
> u.v = _private(page);
> diff --git a/drivers/staging/erofs/xattr.h b/drivers/staging/erofs/xattr.h
> index 35ba5ac..3990805 100644
> --- a/drivers/staging/erofs/xattr.h
> +++ b/drivers/staging/erofs/xattr.h
> @@ -20,14 +20,14 @@
> /* Attribute not found */
> #define ENOATTR ENODATA
>
> -static inline unsigned inlinexattr_header_size(struct inode *inode)
> +static inline unsigned int
[PATCH] staging: erofs:converting all 'unsigned' to 'unsigned int'
Fixed checkpatch warnings: converting all 'unsigned' to 'unsigned int'
Signed-off-by: Pratik Shinde
---
drivers/staging/erofs/internal.h | 7 ---
drivers/staging/erofs/unzip_pagevec.h | 11 ++-
drivers/staging/erofs/unzip_vle.h | 8
drivers/staging/erofs/xattr.h | 17 +
4 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 963cc1b..0ebc294 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -359,8 +359,8 @@ struct erofs_vnode {
unsigned char inode_isize;
unsigned short xattr_isize;
- unsigned xattr_shared_count;
- unsigned *xattr_shared_xattrs;
+ unsigned int xattr_shared_count;
+ unsigned int *xattr_shared_xattrs;
union {
erofs_blk_t raw_blkaddr;
@@ -510,7 +510,8 @@ erofs_grab_bio(struct super_block *sb,
return bio;
}
-static inline void __submit_bio(struct bio *bio, unsigned op, unsigned
op_flags)
+static inline void __submit_bio(struct bio *bio, unsigned int op,
+ unsigned int op_flags)
{
bio_set_op_attrs(bio, op, op_flags);
submit_bio(bio);
diff --git a/drivers/staging/erofs/unzip_pagevec.h
b/drivers/staging/erofs/unzip_pagevec.h
index 7af0ba8..e65dbca 100644
--- a/drivers/staging/erofs/unzip_pagevec.h
+++ b/drivers/staging/erofs/unzip_pagevec.h
@@ -54,9 +54,9 @@ static inline void z_erofs_pagevec_ctor_exit(struct
z_erofs_pagevec_ctor *ctor,
static inline struct page *
z_erofs_pagevec_ctor_next_page(struct z_erofs_pagevec_ctor *ctor,
- unsigned nr)
+ unsigned int nr)
{
- unsigned index;
+ unsigned int index;
/* keep away from occupied pages */
if (ctor->next)
@@ -64,7 +64,7 @@ z_erofs_pagevec_ctor_next_page(struct z_erofs_pagevec_ctor
*ctor,
for (index = 0; index < nr; ++index) {
const erofs_vtptr_t t = ctor->pages[index];
- const unsigned tags = tagptr_unfold_tags(t);
+ const unsigned int tags = tagptr_unfold_tags(t);
if (tags == Z_EROFS_PAGE_TYPE_EXCLUSIVE)
return tagptr_unfold_ptr(t);
@@ -91,8 +91,9 @@ z_erofs_pagevec_ctor_pagedown(struct z_erofs_pagevec_ctor
*ctor,
}
static inline void z_erofs_pagevec_ctor_init(struct z_erofs_pagevec_ctor *ctor,
-unsigned nr,
-erofs_vtptr_t *pages, unsigned i)
+unsigned int nr,
+erofs_vtptr_t *pages,
+unsigned int i)
{
ctor->nr = nr;
ctor->curr = ctor->next = NULL;
diff --git a/drivers/staging/erofs/unzip_vle.h
b/drivers/staging/erofs/unzip_vle.h
index ab509d75..df91ad1 100644
--- a/drivers/staging/erofs/unzip_vle.h
+++ b/drivers/staging/erofs/unzip_vle.h
@@ -34,7 +34,7 @@ struct z_erofs_vle_work {
unsigned short nr_pages;
/* L: queued pages in pagevec[] */
- unsigned vcnt;
+ unsigned int vcnt;
union {
/* L: pagevec */
@@ -124,7 +124,7 @@ union z_erofs_onlinepage_converter {
unsigned long *v;
};
-static inline unsigned z_erofs_onlinepage_index(struct page *page)
+static inline unsigned int z_erofs_onlinepage_index(struct page *page)
{
union z_erofs_onlinepage_converter u;
@@ -164,7 +164,7 @@ static inline void z_erofs_onlinepage_fixup(struct page
*page,
}
v = (index << Z_EROFS_ONLINEPAGE_INDEX_SHIFT) |
- ((o & Z_EROFS_ONLINEPAGE_COUNT_MASK) + (unsigned)down);
+ ((o & Z_EROFS_ONLINEPAGE_COUNT_MASK) + (unsigned int)down);
if (cmpxchg(p, o, v) != o)
goto repeat;
}
@@ -172,7 +172,7 @@ static inline void z_erofs_onlinepage_fixup(struct page
*page,
static inline void z_erofs_onlinepage_endio(struct page *page)
{
union z_erofs_onlinepage_converter u;
- unsigned v;
+ unsigned int v;
DBG_BUGON(!PagePrivate(page));
u.v = _private(page);
diff --git a/drivers/staging/erofs/xattr.h b/drivers/staging/erofs/xattr.h
index 35ba5ac..3990805 100644
--- a/drivers/staging/erofs/xattr.h
+++ b/drivers/staging/erofs/xattr.h
@@ -20,14 +20,14 @@
/* Attribute not found */
#define ENOATTR ENODATA
-static inline unsigned inlinexattr_header_size(struct inode *inode)
+static inline unsigned int inlinexattr_header_size(struct inode *inode)
{
return sizeof(struct erofs_xattr_ibody_header)
+ sizeof(u32) * EROFS_V(inode)->xattr_shared_count;
}
-static inline erofs_blk_t
-xattrblock_addr(struct erofs_sb_info *sbi, unsigned xattr_id)
+static inline erofs_blk_t xattrblock_addr(struct erofs_sb_info *sbi,
+
Re: [PATCH v3] staging: erofs:converting all 'unsigned' to 'unsigned int'
On 2019/7/15 18:43, Pratik Shinde wrote: > Fixed check patch warnings: converting all 'unsigned' to 'unsigned int' > > Signed-off-by: Pratik Shinde oops, should be "staging: erofs: converting all 'unsigned' to 'unsigned int'".. ^ My fault... maybe Greg could fix this issue while merging if accepted. Apart from that, it looks good to me, Reviewed-by: Gao Xiang Thanks, Gao Xiang ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Kindly Respond
Hello, I am Barr Fredrick Mbogo a business consultant i have a lucrative business to discuss with you from the Eastern part of Africa Uganda to be precise aimed at agreed percentage upon your acceptance of my hand in business and friendship. Kindly respond to me if you are interested to partner with me for an update. Very important. Yours Sincerely, Donald Douglas, For, Barr Frederick Mbogo Legal Consultant. Reply to: barrfredmb...@consultant.com ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3] staging: erofs:converting all 'unsigned' to 'unsigned int'
Fixed check patch warnings: converting all 'unsigned' to 'unsigned int'
Signed-off-by: Pratik Shinde
---
drivers/staging/erofs/internal.h | 7 ---
drivers/staging/erofs/unzip_pagevec.h | 11 ++-
drivers/staging/erofs/unzip_vle.h | 8
drivers/staging/erofs/xattr.h | 15 ---
4 files changed, 22 insertions(+), 19 deletions(-)
diff --git a/drivers/staging/erofs/internal.h b/drivers/staging/erofs/internal.h
index 963cc1b..0ebc294 100644
--- a/drivers/staging/erofs/internal.h
+++ b/drivers/staging/erofs/internal.h
@@ -359,8 +359,8 @@ struct erofs_vnode {
unsigned char inode_isize;
unsigned short xattr_isize;
- unsigned xattr_shared_count;
- unsigned *xattr_shared_xattrs;
+ unsigned int xattr_shared_count;
+ unsigned int *xattr_shared_xattrs;
union {
erofs_blk_t raw_blkaddr;
@@ -510,7 +510,8 @@ erofs_grab_bio(struct super_block *sb,
return bio;
}
-static inline void __submit_bio(struct bio *bio, unsigned op, unsigned
op_flags)
+static inline void __submit_bio(struct bio *bio, unsigned int op,
+ unsigned int op_flags)
{
bio_set_op_attrs(bio, op, op_flags);
submit_bio(bio);
diff --git a/drivers/staging/erofs/unzip_pagevec.h
b/drivers/staging/erofs/unzip_pagevec.h
index 7af0ba8..e65dbca 100644
--- a/drivers/staging/erofs/unzip_pagevec.h
+++ b/drivers/staging/erofs/unzip_pagevec.h
@@ -54,9 +54,9 @@ static inline void z_erofs_pagevec_ctor_exit(struct
z_erofs_pagevec_ctor *ctor,
static inline struct page *
z_erofs_pagevec_ctor_next_page(struct z_erofs_pagevec_ctor *ctor,
- unsigned nr)
+ unsigned int nr)
{
- unsigned index;
+ unsigned int index;
/* keep away from occupied pages */
if (ctor->next)
@@ -64,7 +64,7 @@ z_erofs_pagevec_ctor_next_page(struct z_erofs_pagevec_ctor
*ctor,
for (index = 0; index < nr; ++index) {
const erofs_vtptr_t t = ctor->pages[index];
- const unsigned tags = tagptr_unfold_tags(t);
+ const unsigned int tags = tagptr_unfold_tags(t);
if (tags == Z_EROFS_PAGE_TYPE_EXCLUSIVE)
return tagptr_unfold_ptr(t);
@@ -91,8 +91,9 @@ z_erofs_pagevec_ctor_pagedown(struct z_erofs_pagevec_ctor
*ctor,
}
static inline void z_erofs_pagevec_ctor_init(struct z_erofs_pagevec_ctor *ctor,
-unsigned nr,
-erofs_vtptr_t *pages, unsigned i)
+unsigned int nr,
+erofs_vtptr_t *pages,
+unsigned int i)
{
ctor->nr = nr;
ctor->curr = ctor->next = NULL;
diff --git a/drivers/staging/erofs/unzip_vle.h
b/drivers/staging/erofs/unzip_vle.h
index ab509d75..df91ad1 100644
--- a/drivers/staging/erofs/unzip_vle.h
+++ b/drivers/staging/erofs/unzip_vle.h
@@ -34,7 +34,7 @@ struct z_erofs_vle_work {
unsigned short nr_pages;
/* L: queued pages in pagevec[] */
- unsigned vcnt;
+ unsigned int vcnt;
union {
/* L: pagevec */
@@ -124,7 +124,7 @@ union z_erofs_onlinepage_converter {
unsigned long *v;
};
-static inline unsigned z_erofs_onlinepage_index(struct page *page)
+static inline unsigned int z_erofs_onlinepage_index(struct page *page)
{
union z_erofs_onlinepage_converter u;
@@ -164,7 +164,7 @@ static inline void z_erofs_onlinepage_fixup(struct page
*page,
}
v = (index << Z_EROFS_ONLINEPAGE_INDEX_SHIFT) |
- ((o & Z_EROFS_ONLINEPAGE_COUNT_MASK) + (unsigned)down);
+ ((o & Z_EROFS_ONLINEPAGE_COUNT_MASK) + (unsigned int)down);
if (cmpxchg(p, o, v) != o)
goto repeat;
}
@@ -172,7 +172,7 @@ static inline void z_erofs_onlinepage_fixup(struct page
*page,
static inline void z_erofs_onlinepage_endio(struct page *page)
{
union z_erofs_onlinepage_converter u;
- unsigned v;
+ unsigned int v;
DBG_BUGON(!PagePrivate(page));
u.v = _private(page);
diff --git a/drivers/staging/erofs/xattr.h b/drivers/staging/erofs/xattr.h
index 35ba5ac..b9f6e9d 100644
--- a/drivers/staging/erofs/xattr.h
+++ b/drivers/staging/erofs/xattr.h
@@ -20,14 +20,14 @@
/* Attribute not found */
#define ENOATTR ENODATA
-static inline unsigned inlinexattr_header_size(struct inode *inode)
+static inline unsigned int inlinexattr_header_size(struct inode *inode)
{
return sizeof(struct erofs_xattr_ibody_header)
+ sizeof(u32) * EROFS_V(inode)->xattr_shared_count;
}
static inline erofs_blk_t
-xattrblock_addr(struct erofs_sb_info *sbi, unsigned xattr_id)
+xattrblock_addr(struct erofs_sb_info *sbi, unsigned int xattr_id)
{
#ifdef CONFIG_EROFS_FS_XATTR
return
Re: [PATCH v2 00/24] erofs: promote erofs from staging
On 2019/7/15 15:56, Pavel Machek wrote:
> Hi!
>
Changelog from v1:
o resend the whole filesystem into a patchset suggested by Greg;
o code is more cleaner, especially for decompression frontend.
--8<--
Hi,
EROFS file system has been in Linux-staging for about a year.
It has been proved to be stable enough to move out of staging
by 10+ millions of HUAWEI Android mobile phones on the market
from EMUI 9.0.1, and it was promoted as one of the key features
of EMUI 9.1 [1], including P30(pro).
>>>
>>> Ok, maybe it is ready to be moved to kernel proper, but as git can
>>> do moves, would it be better to do it as one commit?
>>>
>>> Separate patches are still better for review, I guess.
>>
>> Thanks for you reply. Either form is OK for me... The first step could
>> be that I hope someone could kindly take some time to look into these
>> patches... :)
>>
>> The patch v2 is slightly different for the current code in the staging
>> tree since I did some code cleanup these days (mainly renaming / moving,
>> including rename unzip_vle.{c,h} to zdata.{c,h} and some confusing
>> structure names and clean up internal.h...). No functional chance and I
>> can submit cleanup patches to staging as well if doing moves by git...
>
> I believe you should get those committed to staging/, yes. Then you
> ask Al Viro to do pull the git move, I guess, and you follow his
> instructions at that point...
>
> FILESYSTEMS (VFS and infrastructure)
> M: Alexander Viro
> L: linux-fsde...@vger.kernel.org
OK, I will send the incremental patches as well later if the above approach
can be done in practice...
Actually I'd like to get fs people Acked-by about EROFS stuffes, e.g. Al, Ted,
etc...
Hello?
It seems rare filesystems upstreamed these years, but I think EROFS is more
useful
after moving out of staging. If some people really care about compression ratio,
I can add multi-block fixed-output compression support later (Not very hard,
it's
already on my TODO list), although my current company HUAWEI doesn't have any
interest in that way in the near future...
In the long term, I'd like to spend my personal free time to decouple code like
fscrypt and introduce fscompr for other generic fs to compress unmodified files
as well then...
That is another stuff. Anyway, EROFS is one of optimal read-only performance
solutions for consumer electronics compared with others (Note that block storage
has been improved a lot in the past decade...)
Thank you very much,
Gao Xiang
>
> Best regards,
> Pavel
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 00/24] erofs: promote erofs from staging
Hi!
> >> Changelog from v1:
> >> o resend the whole filesystem into a patchset suggested by Greg;
> >> o code is more cleaner, especially for decompression frontend.
> >>
> >> --8<--
> >>
> >> Hi,
> >>
> >> EROFS file system has been in Linux-staging for about a year.
> >> It has been proved to be stable enough to move out of staging
> >> by 10+ millions of HUAWEI Android mobile phones on the market
> >> from EMUI 9.0.1, and it was promoted as one of the key features
> >> of EMUI 9.1 [1], including P30(pro).
> >
> > Ok, maybe it is ready to be moved to kernel proper, but as git can
> > do moves, would it be better to do it as one commit?
> >
> > Separate patches are still better for review, I guess.
>
> Thanks for you reply. Either form is OK for me... The first step could
> be that I hope someone could kindly take some time to look into these
> patches... :)
>
> The patch v2 is slightly different for the current code in the staging
> tree since I did some code cleanup these days (mainly renaming / moving,
> including rename unzip_vle.{c,h} to zdata.{c,h} and some confusing
> structure names and clean up internal.h...). No functional chance and I
> can submit cleanup patches to staging as well if doing moves by git...
I believe you should get those committed to staging/, yes. Then you
ask Al Viro to do pull the git move, I guess, and you follow his
instructions at that point...
FILESYSTEMS (VFS and infrastructure)
M: Alexander Viro
L: linux-fsde...@vger.kernel.org
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
signature.asc
Description: Digital signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [v4] staging: most: remove redundant print statement when kfifo_alloc fails
On Sun, Jul 14, 2019 at 11:47:37AM -0400, Keyur Patel wrote: > I didn't get you. I stiil need to update changelog and send more > version or not. If you say so, I can send one more. Please note that the person/bot you are responding to is on in my email blacklist, and many others, so I wouldn't worry too much about the responses. I'll review this patch once 5.3-rc1 is out as I can't do anything with it during the merge window. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] mm/gup: Use put_user_page*() instead of put_page*()
On Sun, Jul 14, 2019 at 08:33:57PM -0600, Jens Axboe wrote:
> On 7/14/19 1:08 PM, Bharath Vedartham wrote:
> > diff --git a/fs/io_uring.c b/fs/io_uring.c
> > index 4ef62a4..b4a4549 100644
> > --- a/fs/io_uring.c
> > +++ b/fs/io_uring.c
> > @@ -2694,10 +2694,9 @@ static int io_sqe_buffer_register(struct io_ring_ctx
> > *ctx, void __user *arg,
> > * if we did partial map, or found file backed vmas,
> > * release any pages we did get
> > */
> > - if (pret > 0) {
> > - for (j = 0; j < pret; j++)
> > - put_page(pages[j]);
> > - }
> > + if (pret > 0)
> > + put_user_pages(pages, pret);
> > +
> > if (ctx->account_mem)
> > io_unaccount_mem(ctx->user, nr_pages);
> > kvfree(imu->bvec);
>
> You handled just the failure case of the buffer registration, but not
> the actual free in io_sqe_buffer_unregister().
>
> --
> Jens Axboe
Yup got it! Thanks! I won't be sending a patch again as fs/io_uring.c
may have larger local changes for put_user_pages.
Thanks
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] mm/gup: Use put_user_page*() instead of put_page*()
On Sun, Jul 14, 2019 at 04:33:42PM -0700, John Hubbard wrote: > On 7/14/19 12:08 PM, Bharath Vedartham wrote: > > This patch converts all call sites of get_user_pages > > to use put_user_page*() instead of put_page*() functions to > > release reference to gup pinned pages. Hi John, > Hi Bharath, > > Thanks for jumping in to help, and welcome to the party! > > You've caught everyone in the middle of a merge window, btw. As a > result, I'm busy rebasing and reworking the get_user_pages call sites, > and gup tracking, in the wake of some semi-traumatic changes to bio > and gup and such. I plan to re-post right after 5.3-rc1 shows up, from > here: > > https://github.com/johnhubbard/linux/commits/gup_dma_core > > ...which you'll find already covers the changes you've posted, except for: > > drivers/misc/sgi-gru/grufault.c > drivers/staging/kpc2000/kpc_dma/fileops.c > > ...and this one, which is undergoing to larger local changes, due to > bvec, so let's leave it out of the choices: > > fs/io_uring.c > > Therefore, until -rc1, if you'd like to help, I'd recommend one or more > of the following ideas: > > 1. Pull down https://github.com/johnhubbard/linux/commits/gup_dma_core > and find missing conversions: look for any additional missing > get_user_pages/put_page conversions. You've already found a couple missing > ones. I haven't re-run a search in a long time, so there's probably even more. > a) And find more, after I rebase to 5.3-rc1: people probably are adding > get_user_pages() calls as we speak. :) Shouldn't this be documented then? I don't see any docs for using put_user_page*() in v5.2.1 in the memory management API section? > 2. Patches: Focus on just one subsystem at a time, and perfect the patch for > it. For example, I think this the staging driver would be perfect to start > with: > > drivers/staging/kpc2000/kpc_dma/fileops.c > > a) verify that you've really, corrected converted the whole > driver. (Hint: I think you might be overlooking a put_page call.) Yup. I did see that! Will fix it! > b) Attempt to test it if you can (I'm being hypocritical in > the extreme here, but one of my problems is that testing > has been light, so any help is very valuable). qemu...? > OTOH, maybe even qemu cannot easily test a kpc2000, but > perhaps `git blame` and talking to the authors would help > figure out a way to validate the changes. Great! I ll do that, I ll mail the patch authors and ask them for help in testing. > Thinking about whether you can run a test that would prove or > disprove my claim in (a), above, could be useful in coming up > with tests to run. > In other words, a few very high quality conversions (even just one) that > we can really put our faith in, is what I value most here. Tested patches > are awesome. I understand that! > 3. Once I re-post, turn on the new CONFIG_DEBUG_GET_USER_PAGES_REFERENCES > and run things such as xfstest/fstest. (Again, doing so would be going > further than I have yet--very helpful). Help clarify what conversions have > actually been tested and work, and which ones remain unvalidated. > Other: Please note that this: Yup will do that. > https://github.com/johnhubbard/linux/commits/gup_dma_core > > a) gets rebased often, and > > b) has a bunch of commits (iov_iter and related) that conflict >with the latest linux.git, > > c) has some bugs in the bio area, that I'm fixing, so I don't trust >that's it's safely runnable, for a few more days. I assume your repo contains only work related to fixing gup issues and not the main repo for gup development? i.e where gup changes are merged? Also are release_pages and put_user_pages interchangable? > One note below, for the future: > > > > > This is a bunch of trivial conversions which is a part of an effort > > by John Hubbard to solve issues with gup pinned pages and > > filesystem writeback. > > > > The issue is more clearly described in John Hubbard's patch[1] where > > put_user_page*() functions are introduced. > > > > Currently put_user_page*() simply does put_page but future implementations > > look to change that once treewide change of put_page callsites to > > put_user_page*() is finished. > > > > The lwn article describing the issue with gup pinned pages and filesystem > > writeback [2]. > > > > This patch has been tested by building and booting the kernel as I don't > > have the required hardware to test the device drivers. > > > > I did not modify gpu/drm drivers which use release_pages instead of > > put_page() to release reference of gup pinned pages as I am not clear > > whether release_pages and put_page are interchangable. > > > > [1] https://lkml.org/lkml/2019/3/26/1396 > > When referring to patches in a commit description, please use the > commit hash, not an external link. See Submitting Patches [1] for details. > > Also, once you figure
Re: [PATCH] staging: erofs:converting all 'unsigned' to 'unsigned int'
On 2019/7/15 13:50, Pratik Shinde wrote:
> Fixed check patch warnings: converting all 'unsigned' to 'unsigned int'
>
> Signed-off-by: Pratik Shinde
The subject line should be better as "[PATCH v2] staging: erofs:converting all
'unsigned' to 'unsigned int'"
> ---
> drivers/staging/erofs/internal.h | 7 ---
> drivers/staging/erofs/unzip_pagevec.h | 11 ++-
> drivers/staging/erofs/unzip_vle.h | 8
> drivers/staging/erofs/xattr.h | 11 ++-
> 4 files changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/staging/erofs/internal.h
> b/drivers/staging/erofs/internal.h
> index 963cc1b..0ebc294 100644
> --- a/drivers/staging/erofs/internal.h
> +++ b/drivers/staging/erofs/internal.h
> @@ -359,8 +359,8 @@ struct erofs_vnode {
> unsigned char inode_isize;
> unsigned short xattr_isize;
>
> - unsigned xattr_shared_count;
> - unsigned *xattr_shared_xattrs;
> + unsigned int xattr_shared_count;
> + unsigned int *xattr_shared_xattrs;
>
> union {
> erofs_blk_t raw_blkaddr;
> @@ -510,7 +510,8 @@ erofs_grab_bio(struct super_block *sb,
> return bio;
> }
>
> -static inline void __submit_bio(struct bio *bio, unsigned op, unsigned
> op_flags)
> +static inline void __submit_bio(struct bio *bio, unsigned int op,
> + unsigned int op_flags)
> {
> bio_set_op_attrs(bio, op, op_flags);
> submit_bio(bio);
> diff --git a/drivers/staging/erofs/unzip_pagevec.h
> b/drivers/staging/erofs/unzip_pagevec.h
> index 7af0ba8..e65dbca 100644
> --- a/drivers/staging/erofs/unzip_pagevec.h
> +++ b/drivers/staging/erofs/unzip_pagevec.h
> @@ -54,9 +54,9 @@ static inline void z_erofs_pagevec_ctor_exit(struct
> z_erofs_pagevec_ctor *ctor,
>
> static inline struct page *
> z_erofs_pagevec_ctor_next_page(struct z_erofs_pagevec_ctor *ctor,
> -unsigned nr)
> +unsigned int nr)
> {
> - unsigned index;
> + unsigned int index;
>
> /* keep away from occupied pages */
> if (ctor->next)
> @@ -64,7 +64,7 @@ z_erofs_pagevec_ctor_next_page(struct z_erofs_pagevec_ctor
> *ctor,
>
> for (index = 0; index < nr; ++index) {
> const erofs_vtptr_t t = ctor->pages[index];
> - const unsigned tags = tagptr_unfold_tags(t);
> + const unsigned int tags = tagptr_unfold_tags(t);
>
> if (tags == Z_EROFS_PAGE_TYPE_EXCLUSIVE)
> return tagptr_unfold_ptr(t);
> @@ -91,8 +91,9 @@ z_erofs_pagevec_ctor_pagedown(struct z_erofs_pagevec_ctor
> *ctor,
> }
>
> static inline void z_erofs_pagevec_ctor_init(struct z_erofs_pagevec_ctor
> *ctor,
> - unsigned nr,
> - erofs_vtptr_t *pages, unsigned i)
> + unsigned int nr,
> + erofs_vtptr_t *pages,
> + unsigned int i)
> {
> ctor->nr = nr;
> ctor->curr = ctor->next = NULL;
> diff --git a/drivers/staging/erofs/unzip_vle.h
> b/drivers/staging/erofs/unzip_vle.h
> index ab509d75..df91ad1 100644
> --- a/drivers/staging/erofs/unzip_vle.h
> +++ b/drivers/staging/erofs/unzip_vle.h
> @@ -34,7 +34,7 @@ struct z_erofs_vle_work {
> unsigned short nr_pages;
>
> /* L: queued pages in pagevec[] */
> - unsigned vcnt;
> + unsigned int vcnt;
>
> union {
> /* L: pagevec */
> @@ -124,7 +124,7 @@ union z_erofs_onlinepage_converter {
> unsigned long *v;
> };
>
> -static inline unsigned z_erofs_onlinepage_index(struct page *page)
> +static inline unsigned int z_erofs_onlinepage_index(struct page *page)
> {
> union z_erofs_onlinepage_converter u;
>
> @@ -164,7 +164,7 @@ static inline void z_erofs_onlinepage_fixup(struct page
> *page,
> }
>
> v = (index << Z_EROFS_ONLINEPAGE_INDEX_SHIFT) |
> - ((o & Z_EROFS_ONLINEPAGE_COUNT_MASK) + (unsigned)down);
> + ((o & Z_EROFS_ONLINEPAGE_COUNT_MASK) + (unsigned int)down);
> if (cmpxchg(p, o, v) != o)
> goto repeat;
> }
> @@ -172,7 +172,7 @@ static inline void z_erofs_onlinepage_fixup(struct page
> *page,
> static inline void z_erofs_onlinepage_endio(struct page *page)
> {
> union z_erofs_onlinepage_converter u;
> - unsigned v;
> + unsigned int v;
>
> DBG_BUGON(!PagePrivate(page));
> u.v = _private(page);
> diff --git a/drivers/staging/erofs/xattr.h b/drivers/staging/erofs/xattr.h
> index 35ba5ac..bbf13c4 100644
> --- a/drivers/staging/erofs/xattr.h
> +++ b/drivers/staging/erofs/xattr.h
> @@ -20,14 +20,14 @@
> /* Attribute not found */
> #define ENOATTR ENODATA
>
> -static inline unsigned inlinexattr_header_size(struct inode *inode)
> +static inline unsigned int inlinexattr_header_size(struct inode *inode)
> {
>
