Re: [PATCH v2] staging: atomisp: use kstrdup to replace kmalloc and memcpy
On 10 July 2017 at 01:22, Sakari Ailus wrote:
> On Sun, Jul 09, 2017 at 05:56:15PM +0530, hari prasath wrote:
>> On 8 July 2017 at 16:31, Sakari Ailus wrote:
>> > Hi Hari,
>> >
>> > On Fri, Jul 07, 2017 at 08:15:21PM +0530, Hari Prasath wrote:
>> >> kstrdup kernel primitive can be used to replace kmalloc followed by
>> >> string copy. This was reported by coccinelle tool
>> >>
>> >> Signed-off-by: Hari Prasath
>> >> ---
>> >> .../media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c | 10
>> >> +++---
>> >> 1 file changed, 3 insertions(+), 7 deletions(-)
>> >>
>> >> diff --git
>> >> a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
>> >> b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
>> >> index 34cc56f..68db87b 100644
>> >> --- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
>> >> +++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
>> >> @@ -144,14 +144,10 @@ sh_css_load_blob_info(const char *fw, const struct
>> >> ia_css_fw_info *bi, struct ia
>> >> )
>> >> {
>> >> char *namebuffer;
>> >> - int namelength = (int)strlen(name);
>> >> -
>> >> - namebuffer = (char *) kmalloc(namelength + 1, GFP_KERNEL);
>> >> - if (namebuffer == NULL)
>> >> - return IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY;
>> >> -
>> >> - memcpy(namebuffer, name, namelength + 1);
>> >>
>> >> + namebuffer = kstrdup(name, GFP_KERNEL);
>> >> + if (!namebuffer)
>> >> + return -ENOMEM;
>> >
>> > The patch also changes the return value in error cases. I believe the
>> > caller(s) expect to get errors in the IA_CCS_ERR_* range.
>>
>> Hi,
>>
>> In this particular case, the calling function just checks if it's not
>> success defined by a enum. I think returning -ENOMEM would not effect,
>> at least in this case.
>
> It might not, but the function now returns both negative Posix and positive
> CSS error codes. The CSS error codes could well be converted to Posix but
> it should be done consistently and preferrably in a separate patch.
Hi Sakari, Thanks for your comments. I will stick with just replacing
with kstrdup and retain the original error return value. I will send a
v3.
Regards,
Hari
>
> --
> Sakari Ailus
> e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
Regards,
G.E.Hari Prasath
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3] staging: atomisp: use kstrdup to replace kmalloc and memcpy
kstrdup kernel primitive can be used to replace kmalloc followed by
string copy. This was reported by coccinelle tool.
Signed-off-by: Hari Prasath
---
v1: Replace kmalloc followed by memcpy with kmemdup. Based on
review comments from Alan Cox, this could better be done
using kstrdup.
v2: Replace kmalloc followed by memcpy by kstrdup in this case
as it essentially is a string copy.Review comment recieved
questioning the return value in case of error. Error value
returned should be what the calling function is expecting.
v3: Retain the original error value returned to the calling
function if kstrdup() fails.
---
.../staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
index 34cc56f..5d231ee 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
@@ -144,14 +144,10 @@ sh_css_load_blob_info(const char *fw, const struct
ia_css_fw_info *bi, struct ia
)
{
char *namebuffer;
- int namelength = (int)strlen(name);
- namebuffer = (char *) kmalloc(namelength + 1, GFP_KERNEL);
- if (namebuffer == NULL)
+ namebuffer = kstrdup(name, GFP_KERNEL);
+ if (!namebuffer)
return IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY;
-
- memcpy(namebuffer, name, namelength + 1);
-
bd->name = fw_minibuffer[index].name = namebuffer;
} else {
bd->name = name;
--
2.10.0.GIT
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [greybus-dev] [PATCH v2] staging: greybus: arche: wrap over-length lines
Hi Mitchell,
On 09-07-17, 20:25, Mitchell Tasman wrote:
> Adjust formatting of various statements to keep line length within
> the 80 column limit preferred by the Linux kernel coding style.
We try to follow that most of the time, but the end result should be easily
readable. If it isn't, then we just ignore the rule.
> Signed-off-by: Mitchell Tasman
> ---
> Changes in v2: Add back a missing space in a comment
>
> drivers/staging/greybus/arche-platform.c | 29 +++--
> 1 file changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/greybus/arche-platform.c
> b/drivers/staging/greybus/arche-platform.c
> index eced2d2..eedd239 100644
> --- a/drivers/staging/greybus/arche-platform.c
> +++ b/drivers/staging/greybus/arche-platform.c
> @@ -172,15 +172,21 @@ static irqreturn_t arche_platform_wd_irq(int irq, void
> *devid)
> if (arche_pdata->wake_detect_state == WD_STATE_BOOT_INIT) {
> if (time_before(jiffies,
> arche_pdata->wake_detect_start +
> -
> msecs_to_jiffies(WD_COLDBOOT_PULSE_WIDTH_MS))) {
> -
> arche_platform_set_wake_detect_state(arche_pdata,
> -
> WD_STATE_IDLE);
> + msecs_to_jiffies(
> + WD_COLDBOOT_PULSE_WIDTH_MS))) {
> + arche_platform_set_wake_detect_state(
We don't break the lines like this in kernel to satisfy the 80 column width
rule. Surely, some places would have similar code, but in general this isn't
recommended. And that's why we never bothered about 80 column rule in this and
below cases.
> + arche_pdata,
> + WD_STATE_IDLE);
> } else {
> - /* Check we are not in middle of irq thread
> already */
> + /*
> + * Check we are not in middle of irq thread
> + * already
> + */
Yes, such changes would be fine.
> if (arche_pdata->wake_detect_state !=
> WD_STATE_COLDBOOT_START) {
> -
> arche_platform_set_wake_detect_state(arche_pdata,
> -
> WD_STATE_COLDBOOT_TRIG);
> + arche_platform_set_wake_detect_state(
Not this.
> + arche_pdata,
> + WD_STATE_COLDBOOT_TRIG);
> spin_unlock_irqrestore(
> &arche_pdata->wake_lock,
> flags);
> @@ -199,8 +205,9 @@ static irqreturn_t arche_platform_wd_irq(int irq, void
> *devid)
>* beyond 30msec, then it is coldboot else fallback
>* to standby boot.
>*/
> - arche_platform_set_wake_detect_state(arche_pdata,
> -
> WD_STATE_BOOT_INIT);
> + arche_platform_set_wake_detect_state(
Not this.
> + arche_pdata,
> + WD_STATE_BOOT_INIT);
> }
> }
>
> @@ -657,12 +664,14 @@ static SIMPLE_DEV_PM_OPS(arche_platform_pm_ops,
> arche_platform_resume);
>
> static const struct of_device_id arche_platform_of_match[] = {
> - { .compatible = "google,arche-platform", }, /* Use PID/VID of SVC
> device */
> + /* Use PID/VID of SVC device */
> + { .compatible = "google,arche-platform", },
This would be fine though.
> { },
> };
>
> static const struct of_device_id arche_combined_id[] = {
> - { .compatible = "google,arche-platform", }, /* Use PID/VID of SVC
> device */
> + /* Use PID/VID of SVC device */
> + { .compatible = "google,arche-platform", },
> { .compatible = "usb,2", },
> { },
> };
--
viresh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: greybus: arche: wrap over-length lines
Adjust formatting of various statements to keep line length within
the 80 column limit preferred by the Linux kernel coding style.
Signed-off-by: Mitchell Tasman
---
Changes in v2: Add back a missing space in a comment
drivers/staging/greybus/arche-platform.c | 29 +++--
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/greybus/arche-platform.c
b/drivers/staging/greybus/arche-platform.c
index eced2d2..eedd239 100644
--- a/drivers/staging/greybus/arche-platform.c
+++ b/drivers/staging/greybus/arche-platform.c
@@ -172,15 +172,21 @@ static irqreturn_t arche_platform_wd_irq(int irq, void
*devid)
if (arche_pdata->wake_detect_state == WD_STATE_BOOT_INIT) {
if (time_before(jiffies,
arche_pdata->wake_detect_start +
-
msecs_to_jiffies(WD_COLDBOOT_PULSE_WIDTH_MS))) {
-
arche_platform_set_wake_detect_state(arche_pdata,
-
WD_STATE_IDLE);
+ msecs_to_jiffies(
+ WD_COLDBOOT_PULSE_WIDTH_MS))) {
+ arche_platform_set_wake_detect_state(
+ arche_pdata,
+ WD_STATE_IDLE);
} else {
- /* Check we are not in middle of irq thread
already */
+ /*
+* Check we are not in middle of irq thread
+* already
+*/
if (arche_pdata->wake_detect_state !=
WD_STATE_COLDBOOT_START) {
-
arche_platform_set_wake_detect_state(arche_pdata,
-
WD_STATE_COLDBOOT_TRIG);
+ arche_platform_set_wake_detect_state(
+ arche_pdata,
+ WD_STATE_COLDBOOT_TRIG);
spin_unlock_irqrestore(
&arche_pdata->wake_lock,
flags);
@@ -199,8 +205,9 @@ static irqreturn_t arche_platform_wd_irq(int irq, void
*devid)
* beyond 30msec, then it is coldboot else fallback
* to standby boot.
*/
- arche_platform_set_wake_detect_state(arche_pdata,
-
WD_STATE_BOOT_INIT);
+ arche_platform_set_wake_detect_state(
+ arche_pdata,
+ WD_STATE_BOOT_INIT);
}
}
@@ -657,12 +664,14 @@ static SIMPLE_DEV_PM_OPS(arche_platform_pm_ops,
arche_platform_resume);
static const struct of_device_id arche_platform_of_match[] = {
- { .compatible = "google,arche-platform", }, /* Use PID/VID of SVC
device */
+ /* Use PID/VID of SVC device */
+ { .compatible = "google,arche-platform", },
{ },
};
static const struct of_device_id arche_combined_id[] = {
- { .compatible = "google,arche-platform", }, /* Use PID/VID of SVC
device */
+ /* Use PID/VID of SVC device */
+ { .compatible = "google,arche-platform", },
{ .compatible = "usb,2", },
{ },
};
--
2.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH]
> -Original Message-
> From: armetallica [mailto:armetall...@gmail.com]
> Sent: Sunday, July 9, 2017 7:58 PM
> To: gre...@linuxfoundation.org
> Cc: *S-Par-Maintainer ;
> de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org
> Subject: [PATCH]
>
> From bb1aac6ae6b21b903d8743712e21aeb1a6b22163 Mon Sep 17 00:00:00
> 2001
> From: Armin Schoenlieb
> Date: Mon, 10 Jul 2017 01:52:41 +0200
> Subject: [PATCH] staging: unisys: visorbus: fix brace coding style issue in
> visorbus_main.c This is a patch to the visorbus_main.c file that fixes up six
> brace errors found by the checkpatch.pl tool
>
Thanks for the patch! It looks like I sent up a patch to Greg that addresses
this
issue on 6/30/2017:
[PATCH 25/25] staging: unisys: visorbus: fix improper bracket blocks
Also, your patch come in with a subject of just the word [PATCH] and the patch
comment was messed up. I'm not sure how you generated the patch, but you
might want to try it with format-patch and then do a git send-email to yourself
as a dry-run to see if the patch looks OK when you get it.
David Kershner
> Signed-off-by: Armin Schoenlieb
> ---
> drivers/staging/unisys/visorbus/visorbus_main.c | 18 --
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c
> b/drivers/staging/unisys/visorbus/visorbus_main.c
> index 1c785dd19ddd..c56496269fc8 100644
> --- a/drivers/staging/unisys/visorbus/visorbus_main.c
> +++ b/drivers/staging/unisys/visorbus/visorbus_main.c
> @@ -270,7 +270,8 @@ static const struct attribute_group
> *visorbus_channel_groups[] = {
>
> static ssize_t partition_handle_show(struct device *dev,
>struct device_attribute *attr,
> - char *buf) {
> + char *buf)
> +{
> struct visor_device *vdev = to_visor_device(dev);
> u64 handle = visorchannel_get_clientpartition(vdev->visorchannel);
>
> @@ -280,7 +281,8 @@ static DEVICE_ATTR_RO(partition_handle);
>
> static ssize_t partition_guid_show(struct device *dev,
> struct device_attribute *attr,
> -char *buf) {
> +char *buf)
> +{
> struct visor_device *vdev = to_visor_device(dev);
>
> return sprintf(buf, "{%pUb}\n", &vdev->partition_uuid);
> @@ -289,7 +291,8 @@ static DEVICE_ATTR_RO(partition_guid);
>
> static ssize_t partition_name_show(struct device *dev,
> struct device_attribute *attr,
> -char *buf) {
> +char *buf)
> +{
> struct visor_device *vdev = to_visor_device(dev);
>
> return sprintf(buf, "%s\n", vdev->name);
> @@ -298,7 +301,8 @@ static DEVICE_ATTR_RO(partition_name);
>
> static ssize_t channel_addr_show(struct device *dev,
>struct device_attribute *attr,
> - char *buf) {
> + char *buf)
> +{
> struct visor_device *vdev = to_visor_device(dev);
> u64 addr = visorchannel_get_physaddr(vdev->visorchannel);
>
> @@ -308,7 +312,8 @@ static DEVICE_ATTR_RO(channel_addr);
>
> static ssize_t channel_bytes_show(struct device *dev,
> struct device_attribute *attr,
> - char *buf) {
> + char *buf)
> +{
> struct visor_device *vdev = to_visor_device(dev);
> u64 nbytes = visorchannel_get_nbytes(vdev->visorchannel);
>
> @@ -318,7 +323,8 @@ static DEVICE_ATTR_RO(channel_bytes);
>
> static ssize_t channel_id_show(struct device *dev,
> struct device_attribute *attr,
> -char *buf) {
> +char *buf)
> +{
> struct visor_device *vdev = to_visor_device(dev);
> int len = 0;
>
> --
> 2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/5] staging: ccree: export symbol immediately following function
From: Tyler Olivieri Patch to fix following checkpatch warning: WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable Signed-off-by: Tyler Olivieri --- drivers/staging/ccree/ssi_fips.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/staging/ccree/ssi_fips.c b/drivers/staging/ccree/ssi_fips.c index fdc40f3..523057c 100644 --- a/drivers/staging/ccree/ssi_fips.c +++ b/drivers/staging/ccree/ssi_fips.c @@ -39,7 +39,6 @@ int ssi_fips_get_state(enum cc_fips_state_t *p_state) return rc; } - EXPORT_SYMBOL(ssi_fips_get_state); /* @@ -57,5 +56,4 @@ int ssi_fips_get_error(enum cc_fips_error *p_err) return rc; } - EXPORT_SYMBOL(ssi_fips_get_error); -- 2.9.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/5] staging: ccree: fix placement of curly braces
From: Tyler Olivieri
Patch to fix checkpatch errors:
ERROR: that open brace { should be on the previous line
ERROR: open brace '{' following function declarations go on the next line
Signed-off-by: Tyler Olivieri
---
drivers/staging/ccree/ssi_buffer_mgr.c | 12 ++---
drivers/staging/ccree/ssi_cipher.c | 6 ++-
drivers/staging/ccree/ssi_driver.c | 5 +-
drivers/staging/ccree/ssi_fips_ll.c| 85 +++---
drivers/staging/ccree/ssi_hash.c | 3 +-
drivers/staging/ccree/ssi_sysfs.c | 3 +-
6 files changed, 40 insertions(+), 74 deletions(-)
diff --git a/drivers/staging/ccree/ssi_buffer_mgr.c
b/drivers/staging/ccree/ssi_buffer_mgr.c
index c05cd67..cbf672b 100644
--- a/drivers/staging/ccree/ssi_buffer_mgr.c
+++ b/drivers/staging/ccree/ssi_buffer_mgr.c
@@ -696,8 +696,7 @@ void ssi_buffer_mgr_unmap_aead_request(
}
if (drvdata->coherent &&
(areq_ctx->gen_ctx.op_type == DRV_CRYPTO_DIRECTION_DECRYPT) &&
- likely(req->src == req->dst))
- {
+ likely(req->src == req->dst)) {
u32 size_to_skip = req->assoclen;
if (areq_ctx->is_gcm4543)
@@ -1134,8 +1133,7 @@ static inline int ssi_buffer_mgr_aead_chain_data(
sg_index += areq_ctx->srcSgl->length;
src_mapped_nents--;
}
- if (unlikely(src_mapped_nents > LLI_MAX_NUM_OF_DATA_ENTRIES))
- {
+ if (unlikely(src_mapped_nents > LLI_MAX_NUM_OF_DATA_ENTRIES)) {
SSI_LOG_ERR("Too many fragments. current %d max %d\n",
src_mapped_nents, LLI_MAX_NUM_OF_DATA_ENTRIES);
return -ENOMEM;
@@ -1177,8 +1175,7 @@ static inline int ssi_buffer_mgr_aead_chain_data(
sg_index += areq_ctx->dstSgl->length;
dst_mapped_nents--;
}
- if (unlikely(dst_mapped_nents > LLI_MAX_NUM_OF_DATA_ENTRIES))
- {
+ if (unlikely(dst_mapped_nents > LLI_MAX_NUM_OF_DATA_ENTRIES)) {
SSI_LOG_ERR("Too many fragments. current %d max %d\n",
dst_mapped_nents, LLI_MAX_NUM_OF_DATA_ENTRIES);
return -ENOMEM;
@@ -1274,8 +1271,7 @@ int ssi_buffer_mgr_map_aead_request(
if (drvdata->coherent &&
(areq_ctx->gen_ctx.op_type == DRV_CRYPTO_DIRECTION_DECRYPT) &&
- likely(req->src == req->dst))
- {
+ likely(req->src == req->dst)) {
u32 size_to_skip = req->assoclen;
if (is_gcm4543)
diff --git a/drivers/staging/ccree/ssi_cipher.c
b/drivers/staging/ccree/ssi_cipher.c
index cd2eafc..4ef0c9b 100644
--- a/drivers/staging/ccree/ssi_cipher.c
+++ b/drivers/staging/ccree/ssi_cipher.c
@@ -68,7 +68,8 @@ struct ssi_ablkcipher_ctx {
static void ssi_ablkcipher_complete(struct device *dev, void *ssi_req, void
__iomem *cc_base);
-static int validate_keys_sizes(struct ssi_ablkcipher_ctx *ctx_p, u32 size) {
+static int validate_keys_sizes(struct ssi_ablkcipher_ctx *ctx_p, u32 size)
+{
switch (ctx_p->flow_mode) {
case S_DIN_to_AES:
switch (size) {
@@ -108,7 +109,8 @@ static int validate_keys_sizes(struct ssi_ablkcipher_ctx
*ctx_p, u32 size) {
return -EINVAL;
}
-static int validate_data_size(struct ssi_ablkcipher_ctx *ctx_p, unsigned int
size) {
+static int validate_data_size(struct ssi_ablkcipher_ctx *ctx_p, unsigned int
size)
+{
switch (ctx_p->flow_mode) {
case S_DIN_to_AES:
switch (ctx_p->cipher_mode) {
diff --git a/drivers/staging/ccree/ssi_driver.c
b/drivers/staging/ccree/ssi_driver.c
index 78709b92..d396474 100644
--- a/drivers/staging/ccree/ssi_driver.c
+++ b/drivers/staging/ccree/ssi_driver.c
@@ -301,13 +301,10 @@ static int init_cc_resources(struct platform_device
*plat_dev)
goto init_cc_res_err;
if (!new_drvdata->plat_dev->dev.dma_mask)
- {
new_drvdata->plat_dev->dev.dma_mask =
&new_drvdata->plat_dev->dev.coherent_dma_mask;
- }
+
if (!new_drvdata->plat_dev->dev.coherent_dma_mask)
- {
new_drvdata->plat_dev->dev.coherent_dma_mask =
DMA_BIT_MASK(DMA_BIT_MASK_LEN);
- }
/* Verify correct mapping */
signature_val = CC_HAL_READ_REGISTER(CC_REG_OFFSET(HOST_RGF,
HOST_SIGNATURE));
diff --git a/drivers/staging/ccree/ssi_fips_ll.c
b/drivers/staging/ccree/ssi_fips_ll.c
index 3557e20..4a7a1a6 100644
--- a/drivers/staging/ccree/ssi_fips_ll.c
+++ b/drivers/staging/ccree/ssi_fips_ll.c
@@ -270,8 +270,7 @@ static const FipsGcmData FipsGcmDataTable[] = {
static inline enum cc_fips_error
FIPS_CipherToFipsError(enum drv_cipher_mode mode, bool is_aes)
{
- switch (mode)
- {
+ switch (mode) {
case DRV_CIPHER_ECB:
return is_aes ? CC_REE_FIPS_ERROR_AES_ECB_PUT :
CC_REE_FIPS_ERROR_DES_ECB_PUT;
case DRV_CIPHER_CBC:
@@ -422,8 +421,7 @@ ssi_cipher_fips_power_up_tests(struct ssi_
[PATCH 0/5] staging: ccree: fix checkpatch errors
From: Tyler Olivieri
This patchset fixes several checkpatch errors and warnings in /staging/ccree:
ERROR: that open brace { should be on the previous line
ERROR: open brace '{' following function declarations go on the next line
WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
ERROR: do not use assignment in if condition
ERROR: switch and case should be at the same indent
WARNING: Statements terminations use 1 semicolon
This is also a submission for the eudyptula challenge.
Tyler Olivieri (5):
staging: ccree: remove redudant semicolons
staging: ccree: fix placement of curly braces
staging: ccree: remove assignement in conditional
staging: ccree: export symbol immediately following function
staging: ccree: fix switch case indentation
drivers/staging/ccree/ssi_buffer_mgr.c | 14 ++
drivers/staging/ccree/ssi_cipher.c | 6 ++-
drivers/staging/ccree/ssi_driver.c | 5 +-
drivers/staging/ccree/ssi_fips.c | 2 -
drivers/staging/ccree/ssi_fips_ll.c| 85 +++---
drivers/staging/ccree/ssi_hash.c | 33 +++--
drivers/staging/ccree/ssi_sysfs.c | 3 +-
7 files changed, 57 insertions(+), 91 deletions(-)
--
2.9.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/5] staging: ccree: remove assignement in conditional
From: Tyler Olivieri
Patch to fix following checkpatch error:
ERROR: do not use assignment in if condition
Signed-off-by: Tyler Olivieri
---
drivers/staging/ccree/ssi_hash.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/ccree/ssi_hash.c b/drivers/staging/ccree/ssi_hash.c
index 7e9f273..c66314f 100644
--- a/drivers/staging/ccree/ssi_hash.c
+++ b/drivers/staging/ccree/ssi_hash.c
@@ -602,7 +602,8 @@ static int ssi_hash_update(struct ahash_req_ctx *state,
return 0;
}
- if (unlikely(rc = ssi_buffer_mgr_map_hash_request_update(ctx->drvdata,
state, src, nbytes, block_size))) {
+ rc = ssi_buffer_mgr_map_hash_request_update(ctx->drvdata, state, src,
nbytes, block_size);
+ if (unlikely(rc)) {
if (rc == 1) {
SSI_LOG_DEBUG(" data size not require HW update %x\n",
nbytes);
@@ -1404,7 +1405,8 @@ static int ssi_mac_update(struct ahash_request *req)
state->xcbc_count++;
- if (unlikely(rc = ssi_buffer_mgr_map_hash_request_update(ctx->drvdata,
state, req->src, req->nbytes, block_size))) {
+ rc = ssi_buffer_mgr_map_hash_request_update(ctx->drvdata, state,
req->src, req->nbytes, block_size);
+ if (unlikely(rc)) {
if (rc == 1) {
SSI_LOG_DEBUG(" data size not require HW update %x\n",
req->nbytes);
--
2.9.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/5] staging: ccree: fix switch case indentation
From: Tyler Olivieri
Patch to fix following checkpatch error:
ERROR: switch and case should be at the same indent
Signed-off-by: Tyler Olivieri
---
drivers/staging/ccree/ssi_hash.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/ccree/ssi_hash.c b/drivers/staging/ccree/ssi_hash.c
index c66314f..4f596eb 100644
--- a/drivers/staging/ccree/ssi_hash.c
+++ b/drivers/staging/ccree/ssi_hash.c
@@ -1177,12 +1177,12 @@ static int ssi_xcbc_setkey(struct crypto_ahash *ahash,
CHECK_AND_RETURN_UPON_FIPS_ERROR();
switch (keylen) {
- case AES_KEYSIZE_128:
- case AES_KEYSIZE_192:
- case AES_KEYSIZE_256:
- break;
- default:
- return -EINVAL;
+ case AES_KEYSIZE_128:
+ case AES_KEYSIZE_192:
+ case AES_KEYSIZE_256:
+ break;
+ default:
+ return -EINVAL;
}
ctx->key_params.keylen = keylen;
@@ -1265,12 +1265,12 @@ static int ssi_cmac_setkey(struct crypto_ahash *ahash,
ctx->is_hmac = true;
switch (keylen) {
- case AES_KEYSIZE_128:
- case AES_KEYSIZE_192:
- case AES_KEYSIZE_256:
- break;
- default:
- return -EINVAL;
+ case AES_KEYSIZE_128:
+ case AES_KEYSIZE_192:
+ case AES_KEYSIZE_256:
+ break;
+ default:
+ return -EINVAL;
}
ctx->key_params.keylen = keylen;
--
2.9.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/5] staging: ccree: remove redudant semicolons
From: Tyler Olivieri
Patch to remove checkpatch warning:
WARNING: Statements terminations use 1 semicolon
Signed-off-by: Tyler Olivieri
---
drivers/staging/ccree/ssi_buffer_mgr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/ccree/ssi_buffer_mgr.c
b/drivers/staging/ccree/ssi_buffer_mgr.c
index b35871e..c05cd67 100644
--- a/drivers/staging/ccree/ssi_buffer_mgr.c
+++ b/drivers/staging/ccree/ssi_buffer_mgr.c
@@ -150,7 +150,7 @@ static inline int ssi_buffer_mgr_render_buff_to_mlli(
u32 **mlli_entry_pp)
{
u32 *mlli_entry_p = *mlli_entry_pp;
- u32 new_nents;;
+ u32 new_nents;
/* Verify there is no memory overflow*/
new_nents = (*curr_nents + buff_size / CC_MAX_MLLI_ENTRY_SIZE + 1);
--
2.9.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH]
>From bb1aac6ae6b21b903d8743712e21aeb1a6b22163 Mon Sep 17 00:00:00 2001
From: Armin Schoenlieb
Date: Mon, 10 Jul 2017 01:52:41 +0200
Subject: [PATCH] staging: unisys: visorbus: fix brace coding style issue in
visorbus_main.c This is a patch to the visorbus_main.c file that fixes up six
brace errors found by the checkpatch.pl tool
Signed-off-by: Armin Schoenlieb
---
drivers/staging/unisys/visorbus/visorbus_main.c | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c
b/drivers/staging/unisys/visorbus/visorbus_main.c
index 1c785dd19ddd..c56496269fc8 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -270,7 +270,8 @@ static const struct attribute_group
*visorbus_channel_groups[] = {
static ssize_t partition_handle_show(struct device *dev,
struct device_attribute *attr,
-char *buf) {
+char *buf)
+{
struct visor_device *vdev = to_visor_device(dev);
u64 handle = visorchannel_get_clientpartition(vdev->visorchannel);
@@ -280,7 +281,8 @@ static DEVICE_ATTR_RO(partition_handle);
static ssize_t partition_guid_show(struct device *dev,
struct device_attribute *attr,
- char *buf) {
+ char *buf)
+{
struct visor_device *vdev = to_visor_device(dev);
return sprintf(buf, "{%pUb}\n", &vdev->partition_uuid);
@@ -289,7 +291,8 @@ static DEVICE_ATTR_RO(partition_guid);
static ssize_t partition_name_show(struct device *dev,
struct device_attribute *attr,
- char *buf) {
+ char *buf)
+{
struct visor_device *vdev = to_visor_device(dev);
return sprintf(buf, "%s\n", vdev->name);
@@ -298,7 +301,8 @@ static DEVICE_ATTR_RO(partition_name);
static ssize_t channel_addr_show(struct device *dev,
struct device_attribute *attr,
-char *buf) {
+char *buf)
+{
struct visor_device *vdev = to_visor_device(dev);
u64 addr = visorchannel_get_physaddr(vdev->visorchannel);
@@ -308,7 +312,8 @@ static DEVICE_ATTR_RO(channel_addr);
static ssize_t channel_bytes_show(struct device *dev,
struct device_attribute *attr,
- char *buf) {
+ char *buf)
+{
struct visor_device *vdev = to_visor_device(dev);
u64 nbytes = visorchannel_get_nbytes(vdev->visorchannel);
@@ -318,7 +323,8 @@ static DEVICE_ATTR_RO(channel_bytes);
static ssize_t channel_id_show(struct device *dev,
struct device_attribute *attr,
- char *buf) {
+ char *buf)
+{
struct visor_device *vdev = to_visor_device(dev);
int len = 0;
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: wilc1000: fix variable signedness
txq_count receives a value from wilc_wlan_handle_txq()
and therefore should be u32
found using sparse:
drivers/staging/wilc1000/linux_wlan.c:306:58: warning:
incorrect type in argument 2 (different signedness)
Signed-off-by: Ilia Sergachev
---
drivers/staging/wilc1000/linux_wlan.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/wilc1000/linux_wlan.c
b/drivers/staging/wilc1000/linux_wlan.c
index 308708450830..0b097aca597e 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -283,7 +283,8 @@ int wilc_wlan_get_num_conn_ifcs(struct wilc *wilc)
static int linux_wlan_txq_task(void *vp)
{
- int ret, txq_count;
+ int ret;
+ u32 txq_count;
struct wilc_vif *vif;
struct wilc *wl;
struct net_device *dev = vp;
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: greybus: arche: wrap over-length lines
Adjust formatting of various statements to keep line length within
the 80 column limit preferred by the Linux kernel coding style.
Signed-off-by: Mitchell Tasman
---
drivers/staging/greybus/arche-platform.c | 29 +++--
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/greybus/arche-platform.c
b/drivers/staging/greybus/arche-platform.c
index eced2d2..c386c13 100644
--- a/drivers/staging/greybus/arche-platform.c
+++ b/drivers/staging/greybus/arche-platform.c
@@ -172,15 +172,21 @@ static irqreturn_t arche_platform_wd_irq(int irq, void
*devid)
if (arche_pdata->wake_detect_state == WD_STATE_BOOT_INIT) {
if (time_before(jiffies,
arche_pdata->wake_detect_start +
-
msecs_to_jiffies(WD_COLDBOOT_PULSE_WIDTH_MS))) {
-
arche_platform_set_wake_detect_state(arche_pdata,
-
WD_STATE_IDLE);
+ msecs_to_jiffies(
+ WD_COLDBOOT_PULSE_WIDTH_MS))) {
+ arche_platform_set_wake_detect_state(
+ arche_pdata,
+ WD_STATE_IDLE);
} else {
- /* Check we are not in middle of irq thread
already */
+ /*
+* Check we are not in middle of irq thread
+* already
+*/
if (arche_pdata->wake_detect_state !=
WD_STATE_COLDBOOT_START) {
-
arche_platform_set_wake_detect_state(arche_pdata,
-
WD_STATE_COLDBOOT_TRIG);
+ arche_platform_set_wake_detect_state(
+ arche_pdata,
+ WD_STATE_COLDBOOT_TRIG);
spin_unlock_irqrestore(
&arche_pdata->wake_lock,
flags);
@@ -199,8 +205,9 @@ static irqreturn_t arche_platform_wd_irq(int irq, void
*devid)
* beyond 30msec, then it is coldboot else fallback
* to standby boot.
*/
- arche_platform_set_wake_detect_state(arche_pdata,
-
WD_STATE_BOOT_INIT);
+ arche_platform_set_wake_detect_state(
+ arche_pdata,
+ WD_STATE_BOOT_INIT);
}
}
@@ -657,12 +664,14 @@ static SIMPLE_DEV_PM_OPS(arche_platform_pm_ops,
arche_platform_resume);
static const struct of_device_id arche_platform_of_match[] = {
- { .compatible = "google,arche-platform", }, /* Use PID/VID of SVC
device */
+ /* Use PID/VID of SVC device*/
+ { .compatible = "google,arche-platform", },
{ },
};
static const struct of_device_id arche_combined_id[] = {
- { .compatible = "google,arche-platform", }, /* Use PID/VID of SVC
device */
+ /* Use PID/VID of SVC device */
+ { .compatible = "google,arche-platform", },
{ .compatible = "usb,2", },
{ },
};
--
2.9.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH]
>From 043428d63637a6dd8e52449b73dbb8341885d7e4 Mon Sep 17 00:00:00 2001
From: Armin Schoenlieb
Date: Mon, 10 Jul 2017 01:12:52 +0200
Subject: [PATCH] Staging: media: atomisp2: fixed trailing whitespace error in
atomisp_v4l2.c This is a patch to the atomisp_v4l2.c file that fixes up a
trailing whitespace error found by the checkpatch.pl tool
Signed-off-by: Armin Schoenlieb
---
drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
index a543def739fc..05d02ebb6d25 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
@@ -1277,13 +1277,13 @@ static int atomisp_pci_probe(struct pci_dev *dev,
(ATOMISP_HW_REVISION_ISP2400
<< ATOMISP_HW_REVISION_SHIFT) |
ATOMISP_HW_STEPPING_B0;
-#ifdef FIXME
+#ifdef FIXME
if (INTEL_MID_BOARD(3, TABLET, BYT, BLK, PRO, CRV2) ||
INTEL_MID_BOARD(3, TABLET, BYT, BLK, ENG, CRV2)) {
isp->dfs = &dfs_config_byt_cr;
isp->hpll_freq = HPLL_FREQ_2000MHZ;
} else
-#endif
+#endif
{
isp->dfs = &dfs_config_byt;
isp->hpll_freq = HPLL_FREQ_1600MHZ;
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: atomisp: use kstrdup to replace kmalloc and memcpy
On Sun, Jul 09, 2017 at 05:56:15PM +0530, hari prasath wrote:
> On 8 July 2017 at 16:31, Sakari Ailus wrote:
> > Hi Hari,
> >
> > On Fri, Jul 07, 2017 at 08:15:21PM +0530, Hari Prasath wrote:
> >> kstrdup kernel primitive can be used to replace kmalloc followed by
> >> string copy. This was reported by coccinelle tool
> >>
> >> Signed-off-by: Hari Prasath
> >> ---
> >> .../media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c | 10
> >> +++---
> >> 1 file changed, 3 insertions(+), 7 deletions(-)
> >>
> >> diff --git
> >> a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
> >> b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
> >> index 34cc56f..68db87b 100644
> >> --- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
> >> +++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
> >> @@ -144,14 +144,10 @@ sh_css_load_blob_info(const char *fw, const struct
> >> ia_css_fw_info *bi, struct ia
> >> )
> >> {
> >> char *namebuffer;
> >> - int namelength = (int)strlen(name);
> >> -
> >> - namebuffer = (char *) kmalloc(namelength + 1, GFP_KERNEL);
> >> - if (namebuffer == NULL)
> >> - return IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY;
> >> -
> >> - memcpy(namebuffer, name, namelength + 1);
> >>
> >> + namebuffer = kstrdup(name, GFP_KERNEL);
> >> + if (!namebuffer)
> >> + return -ENOMEM;
> >
> > The patch also changes the return value in error cases. I believe the
> > caller(s) expect to get errors in the IA_CCS_ERR_* range.
>
> Hi,
>
> In this particular case, the calling function just checks if it's not
> success defined by a enum. I think returning -ENOMEM would not effect,
> at least in this case.
It might not, but the function now returns both negative Posix and positive
CSS error codes. The CSS error codes could well be converted to Posix but
it should be done consistently and preferrably in a separate patch.
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 1/3] tty: resolve tty contention between kernel and user space
Hi, On Sun, Jul 09, 2017 at 06:04:17PM +0300, Andy Shevchenko wrote: > Above > 1. take mutex > 2. take reference > > Here: > 1. give mutex back > 2. give reference back > > I think we usually see symmetrical calls, i.e. > > 1. give reference back > 2. give mutex back > > So, what did I miss? After we hold the kref, driver won't disappear from underneath us so we don't need tty_mutex just for decrementing the refcount. Thanks, Okash ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] checkpatch: fixed alignment and comment style
On Sun, 2017-07-09 at 19:39 +0200, Philipp Guendisch wrote: > This patch fixed alignment, comment style and one appearance of > misordered constant in an if comparison. > Semantic should not be affected by this patch. Your email subject is wrong. This is not a checkpatch patch. Your subject line should be something like: [PATCH] staging: atomisp2: hmm: Alignment code to open parenthesis And it's probably more likely to be applied if you separate out the two different types of changes you are making into 2 patches. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 9/9] staging: iio: tsl2x7x: check return value from tsl2x7x_invoke_change()
On Thu, 6 Jul 2017 18:56:26 -0400
Brian Masney wrote:
> The return value from tsl2x7x_invoke_change() was not checked in most
> places in the driver. This patch adds the proper error checks. The
> return values inside tsl2x7x_invoke_change() are now checked by
> this patch as well.
>
> Previously, if there was an error turning the chip back on, then the
> driver would attempt to turn the chip off and incorrectly return
> success. The code to power off the chip is removed by this patch
> since we should fail fast.
>
> Signed-off-by: Brian Masney
This went on cleanly even without patch 8 so:
Applied to the togreg branch of iio.git and pushed out as testing.
Another good set Brian, looking forward to the next one!
Jonathan
> ---
> drivers/staging/iio/light/tsl2x7x.c | 55
> +
> 1 file changed, 37 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/staging/iio/light/tsl2x7x.c
> b/drivers/staging/iio/light/tsl2x7x.c
> index 033468d..c00278d 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -771,22 +771,24 @@ int tsl2x7x_invoke_change(struct iio_dev *indio_dev)
> {
> struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> int device_status = chip->tsl2x7x_chip_status;
> + int ret;
>
> mutex_lock(&chip->als_mutex);
> mutex_lock(&chip->prox_mutex);
>
> - if (device_status == TSL2X7X_CHIP_WORKING)
> - tsl2x7x_chip_off(indio_dev);
> -
> - tsl2x7x_chip_on(indio_dev);
> + if (device_status == TSL2X7X_CHIP_WORKING) {
> + ret = tsl2x7x_chip_off(indio_dev);
> + if (ret < 0)
> + goto unlock;
> + }
>
> - if (device_status != TSL2X7X_CHIP_WORKING)
> - tsl2x7x_chip_off(indio_dev);
> + ret = tsl2x7x_chip_on(indio_dev);
>
> +unlock:
> mutex_unlock(&chip->prox_mutex);
> mutex_unlock(&chip->als_mutex);
>
> - return 0;
> + return ret;
> }
>
> static
> @@ -925,6 +927,7 @@ static ssize_t in_illuminance0_target_input_store(struct
> device *dev,
> struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> unsigned long value;
> + int ret;
>
> if (kstrtoul(buf, 0, &value))
> return -EINVAL;
> @@ -932,7 +935,9 @@ static ssize_t in_illuminance0_target_input_store(struct
> device *dev,
> if (value)
> chip->tsl2x7x_settings.als_cal_target = value;
>
> - tsl2x7x_invoke_change(indio_dev);
> + ret = tsl2x7x_invoke_change(indio_dev);
> + if (ret < 0)
> + return ret;
>
> return len;
> }
> @@ -981,7 +986,9 @@ static ssize_t in_intensity0_thresh_period_store(struct
> device *dev,
> dev_info(&chip->client->dev, "%s: als persistence = %d",
>__func__, filter_delay);
>
> - tsl2x7x_invoke_change(indio_dev);
> + ret = tsl2x7x_invoke_change(indio_dev);
> + if (ret < 0)
> + return ret;
>
> return IIO_VAL_INT_PLUS_MICRO;
> }
> @@ -1029,7 +1036,10 @@ static ssize_t
> in_proximity0_thresh_period_store(struct device *dev,
> dev_info(&chip->client->dev, "%s: prox persistence = %d",
>__func__, filter_delay);
>
> - tsl2x7x_invoke_change(indio_dev);
> + ret = tsl2x7x_invoke_change(indio_dev);
> + if (ret < 0)
> + return ret;
> +
>
> return IIO_VAL_INT_PLUS_MICRO;
> }
> @@ -1040,6 +1050,7 @@ static ssize_t in_illuminance0_calibrate_store(struct
> device *dev,
> {
> struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> bool value;
> + int ret;
>
> if (strtobool(buf, &value))
> return -EINVAL;
> @@ -1047,7 +1058,9 @@ static ssize_t in_illuminance0_calibrate_store(struct
> device *dev,
> if (value)
> tsl2x7x_als_calibrate(indio_dev);
>
> - tsl2x7x_invoke_change(indio_dev);
> + ret = tsl2x7x_invoke_change(indio_dev);
> + if (ret < 0)
> + return ret;
>
> return len;
> }
> @@ -1087,7 +1100,7 @@ static ssize_t in_illuminance0_lux_table_store(struct
> device *dev,
> struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> int value[ARRAY_SIZE(chip->tsl2x7x_device_lux) * 3 + 1];
> - int n;
> + int n, ret;
>
> get_options(buf, ARRAY_SIZE(value), value);
>
> @@ -1115,7 +1128,9 @@ static ssize_t in_illuminance0_lux_table_store(struct
> device *dev,
> memset(chip->tsl2x7x_device_lux, 0, sizeof(chip->tsl2x7x_device_lux));
> memcpy(chip->tsl2x7x_device_lux, &value[1], (value[0] * 4));
>
> - tsl2x7x_invoke_change(indio_dev);
> + ret = tsl2x7x_invoke_change(indio_dev);
> + if (ret < 0)
> + return ret;
>
> return len;
> }
> @@ -1126,6 +1141,7 @@ static ssize_t in_proximity0_calibrate_store(struct
> device *dev,
> {
> struct iio_dev *indio_dev = dev_t
[PATCH] checkpatch: fixed alignment and comment style
This patch fixed alignment, comment style and one appearance of
misordered constant in an if comparison.
Semantic should not be affected by this patch.
There are still a few warnings and even errors left which require
a semantic change of the code to fix.
e.g. use of in_atomic in drivers
There are also two warnings left about too long lines, which
reduce readability if changed.
Signed-off-by: Philipp Guendisch
Signed-off-by: Chris Baller
---
.../staging/media/atomisp/pci/atomisp2/hmm/hmm.c | 134 +++--
1 file changed, 68 insertions(+), 66 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c
b/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c
index 5729539..41d3534 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c
@@ -45,14 +45,16 @@ struct hmm_pool reserved_pool;
static ia_css_ptr dummy_ptr;
struct _hmm_mem_stat hmm_mem_stat;
-/* p: private
- s: shared
- u: user
- i: ion */
+/*
+ * p: private
+ * s: shared
+ * u: user
+ * i: ion
+ */
static const char hmm_bo_type_string[] = "psui";
static ssize_t bo_show(struct device *dev, struct device_attribute *attr,
- char *buf, struct list_head *bo_list, bool active)
+ char *buf, struct list_head *bo_list, bool active)
{
ssize_t ret = 0;
struct hmm_buffer_object *bo;
@@ -72,10 +74,10 @@ static ssize_t bo_show(struct device *dev, struct
device_attribute *attr,
spin_lock_irqsave(&bo_device.list_lock, flags);
list_for_each_entry(bo, bo_list, list) {
if ((active && (bo->status & HMM_BO_ALLOCED)) ||
- (!active && !(bo->status & HMM_BO_ALLOCED))) {
+ (!active && !(bo->status & HMM_BO_ALLOCED))) {
ret = scnprintf(buf + index1, PAGE_SIZE - index1,
- "%c %d\n",
- hmm_bo_type_string[bo->type], bo->pgnr);
+ "%c %d\n",
+ hmm_bo_type_string[bo->type], bo->pgnr);
total[bo->type] += bo->pgnr;
count[bo->type]++;
@@ -88,9 +90,10 @@ static ssize_t bo_show(struct device *dev, struct
device_attribute *attr,
for (i = 0; i < HMM_BO_LAST; i++) {
if (count[i]) {
ret = scnprintf(buf + index1 + index2,
- PAGE_SIZE - index1 - index2,
- "%ld %c buffer objects: %ld KB\n",
- count[i], hmm_bo_type_string[i], total[i] * 4);
+ PAGE_SIZE - index1 - index2,
+ "%ld %c buffer objects: %ld KB\n",
+ count[i], hmm_bo_type_string[i],
+ total[i] * 4);
if (ret > 0)
index2 += ret;
}
@@ -101,22 +104,22 @@ static ssize_t bo_show(struct device *dev, struct
device_attribute *attr,
}
static ssize_t active_bo_show(struct device *dev,
- struct device_attribute *attr,
- char *buf)
+ struct device_attribute *attr,
+ char *buf)
{
return bo_show(dev, attr, buf, &bo_device.entire_bo_list, true);
}
static ssize_t free_bo_show(struct device *dev,
- struct device_attribute *attr,
- char *buf)
+ struct device_attribute *attr,
+ char *buf)
{
return bo_show(dev, attr, buf, &bo_device.entire_bo_list, false);
}
static ssize_t reserved_pool_show(struct device *dev,
- struct device_attribute *attr,
- char *buf)
+ struct device_attribute *attr,
+ char *buf)
{
ssize_t ret = 0;
@@ -128,7 +131,7 @@ static ssize_t reserved_pool_show(struct device *dev,
spin_lock_irqsave(&pinfo->list_lock, flags);
ret = scnprintf(buf, PAGE_SIZE, "%d out of %d pages available\n",
- pinfo->index, pinfo->pgnr);
+ pinfo->index, pinfo->pgnr);
spin_unlock_irqrestore(&pinfo->list_lock, flags);
if (ret > 0)
@@ -138,8 +141,8 @@ static ssize_t reserved_pool_show(struct device *dev,
};
static ssize_t dynamic_pool_show(struct device *dev,
- struct device_attribute *attr,
- char *buf)
+struct device_attribute *attr,
+char *buf)
{
ssize_t ret = 0;
@@ -151,7 +154,7 @@ static ssize_t dynamic_pool_show(struct device *dev,
spin_lock_irqsave(&pinfo->list_lock, flags);
ret = scnprintf(buf, PAGE_SIZE, "%d (max %d) pages availabl
Re: [PATCH v2 8/9] staging: iio: tsl2x7x: migrate in_illuminance0_integration_time sysfs attribute to iio_chan_spec
On Thu, 6 Jul 2017 18:56:25 -0400
Brian Masney wrote:
> The driver explicitly creates the in_illuminance0_integration_time sysfs
> attribute outside the IIO core. This attribute is available in the IIO
> core so this patches migrates the attribute to be created by
> the iio_chan_spec.
>
> Signed-off-by: Brian Masney
> ---
> drivers/staging/iio/light/tsl2x7x.c | 68
> +++--
> 1 file changed, 19 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/staging/iio/light/tsl2x7x.c
> b/drivers/staging/iio/light/tsl2x7x.c
> index c38bd64..033468d 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -905,46 +905,6 @@ static ssize_t
> in_proximity0_calibscale_available_show(struct device *dev,
> return snprintf(buf, PAGE_SIZE, "%s\n", "1 2 4 8");
> }
>
> -static ssize_t in_illuminance0_integration_time_show(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - struct tsl2X7X_chip *chip = iio_priv(dev_to_iio_dev(dev));
> - int y, z;
> -
> - y = (TSL2X7X_MAX_TIMER_CNT - (u8)chip->tsl2x7x_settings.als_time) + 1;
> - z = y * TSL2X7X_MIN_ITIME;
> - y /= 1000;
> - z %= 1000;
> -
> - return snprintf(buf, PAGE_SIZE, "%d.%03d\n", y, z);
> -}
> -
> -static ssize_t in_illuminance0_integration_time_store(struct device *dev,
> - struct device_attribute *attr,
> - const char *buf, size_t len)
> -{
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> - struct tsl2x7x_parse_result result;
> - int ret;
> -
> - ret = iio_str_to_fixpoint(buf, 100, &result.integer, &result.fract);
> - if (ret)
> - return ret;
> -
> - result.fract /= 3;
> - chip->tsl2x7x_settings.als_time =
> - TSL2X7X_MAX_TIMER_CNT - (u8)result.fract;
> -
> - dev_info(&chip->client->dev, "%s: als time = %d",
> - __func__, chip->tsl2x7x_settings.als_time);
> -
> - tsl2x7x_invoke_change(indio_dev);
> -
> - return IIO_VAL_INT_PLUS_MICRO;
> -}
> -
> static IIO_CONST_ATTR(in_illuminance0_integration_time_available,
> ".00272 - .696");
>
> @@ -1369,7 +1329,13 @@ static int tsl2x7x_read_raw(struct iio_dev *indio_dev,
> *val = chip->tsl2x7x_settings.als_gain_trim;
> ret = IIO_VAL_INT;
> break;
> -
> + case IIO_CHAN_INFO_INT_TIME:
> + *val = (TSL2X7X_MAX_TIMER_CNT - chip->tsl2x7x_settings.als_time)
> + + 1;
> + *val2 = (*val * TSL2X7X_MIN_ITIME) % 1000;
> + *val /= 1000;
> + ret = IIO_VAL_INT_PLUS_MICRO;
> + break;
> default:
> ret = -EINVAL;
> }
> @@ -1445,7 +1411,13 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
> case IIO_CHAN_INFO_CALIBBIAS:
> chip->tsl2x7x_settings.als_gain_trim = val;
> break;
> + case IIO_CHAN_INFO_INT_TIME:
> + chip->tsl2x7x_settings.als_time =
> + TSL2X7X_MAX_TIMER_CNT - (val2 / 3);
Is that 3 the same one represented by MIN_ITME?
Looks like it so would be easier to review if it was using the
constant.
Thanks,
Jonathan
>
> + dev_info(&chip->client->dev, "%s: als time = %d",
> + __func__, chip->tsl2x7x_settings.als_time);
> + break;
> default:
> return -EINVAL;
> }
> @@ -1459,8 +1431,6 @@ static
> DEVICE_ATTR_RO(in_proximity0_calibscale_available);
>
> static DEVICE_ATTR_RO(in_illuminance0_calibscale_available);
>
> -static DEVICE_ATTR_RW(in_illuminance0_integration_time);
> -
> static DEVICE_ATTR_RW(in_illuminance0_target_input);
>
> static DEVICE_ATTR_WO(in_illuminance0_calibrate);
> @@ -1540,7 +1510,6 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void
> *private)
>
> static struct attribute *tsl2x7x_ALS_device_attrs[] = {
> &dev_attr_in_illuminance0_calibscale_available.attr,
> - &dev_attr_in_illuminance0_integration_time.attr,
>
> &iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
> &dev_attr_in_illuminance0_target_input.attr,
> &dev_attr_in_illuminance0_calibrate.attr,
> @@ -1555,7 +1524,6 @@ static struct attribute *tsl2x7x_PRX_device_attrs[] = {
>
> static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
> &dev_attr_in_illuminance0_calibscale_available.attr,
> - &dev_attr_in_illuminance0_integration_time.attr,
>
> &iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
> &dev_attr_in_illuminance0_target_input.attr,
> &dev_attr_in_illuminance0_calibrate.attr,
> @@ -1572,7 +1540,6 @@ static struct attribute *tsl2x7x_PRX2_device_attrs[] = {
>
> static struct attribute *tsl2
Re: [PATCH v2 7/9] staging: iio: tsl2x7x: use usleep_range() instead of mdelay()
On Thu, 6 Jul 2017 18:56:24 -0400
Brian Masney wrote:
> This driver in some cases can busy wait for upwards of 15ms. Since the
> kernel at this point is not running in atomic context, and is running in
> process context, we can safely use usleep_range() instead. This patch
> changes the two occurrences of mdelay() to usleep_range().
>
> Signed-off-by: Brian Masney
Applied. Thanks,
Jonathan
> ---
> drivers/staging/iio/light/tsl2x7x.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/iio/light/tsl2x7x.c
> b/drivers/staging/iio/light/tsl2x7x.c
> index d80f5dc..c38bd64 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -686,7 +686,8 @@ static int tsl2x7x_chip_on(struct iio_dev *indio_dev)
> }
> }
>
> - mdelay(3); /* Power-on settling time */
> + /* Power-on settling time */
> + usleep_range(3000, 3500);
>
> /*
>* NOW enable the ADC
> @@ -853,7 +854,7 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
>
> /*gather the samples*/
> for (i = 0; i < chip->tsl2x7x_settings.prox_max_samples_cal; i++) {
> - mdelay(15);
> + usleep_range(15000, 17500);
> tsl2x7x_get_prox(indio_dev);
> prox_history[i] = chip->prox_data;
> dev_info(&chip->client->dev, "2 i=%d prox data= %d\n",
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 6/9] staging: iio: tsl2x7x: refactor {read,write}_event_value to allow handling multiple iio_event_infos
On Thu, 6 Jul 2017 18:56:23 -0400
Brian Masney wrote:
> tsl2x7x_read_thresh() and tsl2x7x_write_thresh() currently assumes
> that IIO_EV_INFO_VALUE is the only iio_event_info that will be
> passed in. This patch refactors these two functions so that
> additional iio_event_infos can be passed in. The functions are
> renamed from tsl2x7x_{read,write}_thresh() to
> tsl2x7x_{read,write}_event_value(). This patch also adds the
> missing return value check to tsl2x7x_invoke_change() since this
> was previously missing. This patch is in preparation for moving
> the in_intensity0_thresh_period and in_proximity0_thresh_period sysfs
> attributes to be created by iio_event_spec.
>
> Signed-off-by: Brian Masney
Applied. Thanks,
Jonathan
> ---
> drivers/staging/iio/light/tsl2x7x.c | 157
> +---
> 1 file changed, 90 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/staging/iio/light/tsl2x7x.c
> b/drivers/staging/iio/light/tsl2x7x.c
> index 5734628..d80f5dc 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -1218,78 +1218,101 @@ static int tsl2x7x_write_interrupt_config(struct
> iio_dev *indio_dev,
> return 0;
> }
>
> -static int tsl2x7x_write_thresh(struct iio_dev *indio_dev,
> - const struct iio_chan_spec *chan,
> - enum iio_event_type type,
> - enum iio_event_direction dir,
> - enum iio_event_info info,
> - int val, int val2)
> +static int tsl2x7x_write_event_value(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + enum iio_event_info info,
> + int val, int val2)
> {
> struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> + int ret = -EINVAL;
>
> - if (chan->type == IIO_INTENSITY) {
> - switch (dir) {
> - case IIO_EV_DIR_RISING:
> - chip->tsl2x7x_settings.als_thresh_high = val;
> - break;
> - case IIO_EV_DIR_FALLING:
> - chip->tsl2x7x_settings.als_thresh_low = val;
> - break;
> - default:
> - return -EINVAL;
> - }
> - } else {
> - switch (dir) {
> - case IIO_EV_DIR_RISING:
> - chip->tsl2x7x_settings.prox_thres_high = val;
> - break;
> - case IIO_EV_DIR_FALLING:
> - chip->tsl2x7x_settings.prox_thres_low = val;
> - break;
> - default:
> - return -EINVAL;
> + switch (info) {
> + case IIO_EV_INFO_VALUE:
> + if (chan->type == IIO_INTENSITY) {
> + switch (dir) {
> + case IIO_EV_DIR_RISING:
> + chip->tsl2x7x_settings.als_thresh_high = val;
> + ret = 0;
> + break;
> + case IIO_EV_DIR_FALLING:
> + chip->tsl2x7x_settings.als_thresh_low = val;
> + ret = 0;
> + break;
> + default:
> + break;
> + }
> + } else {
> + switch (dir) {
> + case IIO_EV_DIR_RISING:
> + chip->tsl2x7x_settings.prox_thres_high = val;
> + ret = 0;
> + break;
> + case IIO_EV_DIR_FALLING:
> + chip->tsl2x7x_settings.prox_thres_low = val;
> + ret = 0;
> + break;
> + default:
> + break;
> + }
> }
> + break;
> + default:
> + break;
> }
>
> - tsl2x7x_invoke_change(indio_dev);
> + if (ret < 0)
> + return ret;
>
> - return 0;
> + return tsl2x7x_invoke_change(indio_dev);
> }
>
> -static int tsl2x7x_read_thresh(struct iio_dev *indio_dev,
> -const struct iio_chan_spec *chan,
> -enum iio_event_type type,
> -enum iio_event_direction dir,
> -enum iio_event_info info,
> -int *val, int *val2)
> +static int tsl2x7x_read_event_value(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_even
Re: [PATCH v2 5/9] staging: iio: tsl2x7x: cleaned up i2c calls in tsl2x7x_als_calibrate()
On Thu, 6 Jul 2017 18:56:22 -0400
Brian Masney wrote:
> The calibration function calls i2c_smbus_write_byte() and
> i2c_smbus_read_byte(). These two function calls are replaced with a
> single call to i2c_smbus_read_byte_data() by this patch. This patch
> also removes an unnecessary call that reads the CNTRL register
> a second time. One of the error paths returned -1 if the ADC was not
> enabled and this patch changes that return value to -EINVAL.
>
> Signed-off-by: Brian Masney
Applied.
> ---
> drivers/staging/iio/light/tsl2x7x.c | 34 --
> 1 file changed, 12 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/staging/iio/light/tsl2x7x.c
> b/drivers/staging/iio/light/tsl2x7x.c
> index 6c9f06b..5734628 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -542,39 +542,29 @@ static void tsl2x7x_defaults(struct tsl2X7X_chip *chip)
> static int tsl2x7x_als_calibrate(struct iio_dev *indio_dev)
> {
> struct tsl2X7X_chip *chip = iio_priv(indio_dev);
> - u8 reg_val;
> int gain_trim_val;
> int ret;
> int lux_val;
>
> - ret = i2c_smbus_write_byte(chip->client,
> -(TSL2X7X_CMD_REG | TSL2X7X_CNTRL));
> + ret = i2c_smbus_read_byte_data(chip->client,
> +TSL2X7X_CMD_REG | TSL2X7X_CNTRL);
> if (ret < 0) {
> dev_err(&chip->client->dev,
> - "failed to write CNTRL register, ret=%d\n", ret);
> + "%s: failed to read from the CNTRL register\n",
> + __func__);
> return ret;
> }
>
> - reg_val = i2c_smbus_read_byte(chip->client);
> - if ((reg_val & (TSL2X7X_CNTL_ADC_ENBL | TSL2X7X_CNTL_PWR_ON))
> - != (TSL2X7X_CNTL_ADC_ENBL | TSL2X7X_CNTL_PWR_ON)) {
> - dev_err(&chip->client->dev,
> - "%s: failed: ADC not enabled\n", __func__);
> - return -1;
> - }
> -
> - ret = i2c_smbus_write_byte(chip->client,
> -(TSL2X7X_CMD_REG | TSL2X7X_CNTRL));
> - if (ret < 0) {
> + if ((ret & (TSL2X7X_CNTL_ADC_ENBL | TSL2X7X_CNTL_PWR_ON))
> + != (TSL2X7X_CNTL_ADC_ENBL | TSL2X7X_CNTL_PWR_ON)) {
> dev_err(&chip->client->dev,
> - "failed to write ctrl reg: ret=%d\n", ret);
> - return ret;
> - }
> -
> - reg_val = i2c_smbus_read_byte(chip->client);
> - if ((reg_val & TSL2X7X_STA_ADC_VALID) != TSL2X7X_STA_ADC_VALID) {
> + "%s: Device is not powered on and/or ADC is not
> enabled\n",
> + __func__);
> + return -EINVAL;
> + } else if ((ret & TSL2X7X_STA_ADC_VALID) != TSL2X7X_STA_ADC_VALID) {
> dev_err(&chip->client->dev,
> - "%s: failed: STATUS - ADC not valid.\n", __func__);
> + "%s: The two ADC channels have not completed an
> integration cycle\n",
> + __func__);
> return -ENODATA;
> }
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 4/9] staging: iio: tsl2x7x: remove tsl2x7x_i2c_read()
On Thu, 6 Jul 2017 18:56:21 -0400
Brian Masney wrote:
> tsl2x7x_i2c_read() would call i2c_smbus_write_byte() and
> i2c_smbus_read_byte(). These two i2c functions can be replaced with a
> single call to i2c_smbus_read_byte_data(). This patch removes the
> tsl2x7x_i2c_read() function and replaces all occurrences with a call to
> i2c_smbus_read_byte_data().
>
> Signed-off-by: Brian Masney
Applied.
> ---
> drivers/staging/iio/light/tsl2x7x.c | 71
> -
> 1 file changed, 22 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/staging/iio/light/tsl2x7x.c
> b/drivers/staging/iio/light/tsl2x7x.c
> index 95ed7cc6..6c9f06b 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -285,35 +285,6 @@ static const u8 device_channel_config[] = {
> };
>
> /**
> - * tsl2x7x_i2c_read() - Read a byte from a register.
> - * @client: i2c client
> - * @reg: device register to read from
> - * @*val:pointer to location to store register contents.
> - *
> - */
> -static int
> -tsl2x7x_i2c_read(struct i2c_client *client, u8 reg, u8 *val)
> -{
> - int ret;
> -
> - /* select register to write */
> - ret = i2c_smbus_write_byte(client, (TSL2X7X_CMD_REG | reg));
> - if (ret < 0) {
> - dev_err(&client->dev, "failed to write register %x\n", reg);
> - return ret;
> - }
> -
> - /* read the data */
> - ret = i2c_smbus_read_byte(client);
> - if (ret >= 0)
> - *val = (u8)ret;
> - else
> - dev_err(&client->dev, "failed to read register %x\n", reg);
> -
> - return ret;
> -}
> -
> -/**
> * tsl2x7x_get_lux() - Reads and calculates current lux value.
> * @indio_dev: pointer to IIO device
> *
> @@ -352,15 +323,15 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
> goto out_unlock;
> }
>
> - ret = tsl2x7x_i2c_read(chip->client,
> -(TSL2X7X_CMD_REG | TSL2X7X_STATUS), &buf[0]);
> + ret = i2c_smbus_read_byte_data(chip->client,
> +TSL2X7X_CMD_REG | TSL2X7X_STATUS);
> if (ret < 0) {
> dev_err(&chip->client->dev,
> "%s: Failed to read STATUS Reg\n", __func__);
> goto out_unlock;
> }
> /* is data new & valid */
> - if (!(buf[0] & TSL2X7X_STA_ADC_VALID)) {
> + if (!(ret & TSL2X7X_STA_ADC_VALID)) {
> dev_err(&chip->client->dev,
> "%s: data not valid yet\n", __func__);
> ret = chip->als_cur_info.lux; /* return LAST VALUE */
> @@ -368,14 +339,16 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
> }
>
> for (i = 0; i < 4; i++) {
> - ret = tsl2x7x_i2c_read(chip->client,
> -(TSL2X7X_CMD_REG |
> -(TSL2X7X_ALS_CHAN0LO + i)), &buf[i]);
> + int reg = TSL2X7X_CMD_REG | (TSL2X7X_ALS_CHAN0LO + i);
> +
> + ret = i2c_smbus_read_byte_data(chip->client, reg);
> if (ret < 0) {
> dev_err(&chip->client->dev,
> "failed to read. err=%x\n", ret);
> goto out_unlock;
> }
> +
> + buf[i] = ret;
> }
>
> /* clear any existing interrupt status */
> @@ -475,7 +448,6 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
> {
> int i;
> int ret;
> - u8 status;
> u8 chdata[2];
> struct tsl2X7X_chip *chip = iio_priv(indio_dev);
>
> @@ -485,8 +457,8 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
> return -EBUSY;
> }
>
> - ret = tsl2x7x_i2c_read(chip->client,
> -(TSL2X7X_CMD_REG | TSL2X7X_STATUS), &status);
> + ret = i2c_smbus_read_byte_data(chip->client,
> +TSL2X7X_CMD_REG | TSL2X7X_STATUS);
> if (ret < 0) {
> dev_err(&chip->client->dev, "i2c err=%d\n", ret);
> goto prox_poll_err;
> @@ -498,7 +470,7 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
> case tmd2671:
> case tsl2771:
> case tmd2771:
> - if (!(status & TSL2X7X_STA_ADC_VALID))
> + if (!(ret & TSL2X7X_STA_ADC_VALID))
> goto prox_poll_err;
> break;
> case tsl2572:
> @@ -506,17 +478,19 @@ static int tsl2x7x_get_prox(struct iio_dev *indio_dev)
> case tmd2672:
> case tsl2772:
> case tmd2772:
> - if (!(status & TSL2X7X_STA_PRX_VALID))
> + if (!(ret & TSL2X7X_STA_PRX_VALID))
> goto prox_poll_err;
> break;
> }
>
> for (i = 0; i < 2; i++) {
> - ret = tsl2x7x_i2c_read(chip->client,
> -(TSL2X7X_CMD_REG |
> -(TSL2X7X_PRX_LO + i)), &chdata[i]);
> +
Re: [PATCH v2 3/9] staging: iio: tsl2x7x: remove redundant power_state sysfs attribute
On Thu, 6 Jul 2017 18:56:20 -0400
Brian Masney wrote:
> The TSL2X7X driver has a custom power_state sysfs attribute. Remove this
> attribute since the runtime power management code provides a sysfs
> attribute to control the power state of the device.
>
> Signed-off-by: Brian Masney
Applied.
> ---
> drivers/staging/iio/light/tsl2x7x.c | 34 --
> 1 file changed, 34 deletions(-)
>
> diff --git a/drivers/staging/iio/light/tsl2x7x.c
> b/drivers/staging/iio/light/tsl2x7x.c
> index 8148986..95ed7cc6 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -915,33 +915,6 @@ static void tsl2x7x_prox_cal(struct iio_dev *indio_dev)
> tsl2x7x_chip_on(indio_dev);
> }
>
> -static ssize_t power_state_show(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - struct tsl2X7X_chip *chip = iio_priv(dev_to_iio_dev(dev));
> -
> - return snprintf(buf, PAGE_SIZE, "%d\n", chip->tsl2x7x_chip_status);
> -}
> -
> -static ssize_t power_state_store(struct device *dev,
> - struct device_attribute *attr,
> - const char *buf, size_t len)
> -{
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - bool value;
> -
> - if (strtobool(buf, &value))
> - return -EINVAL;
> -
> - if (value)
> - tsl2x7x_chip_on(indio_dev);
> - else
> - tsl2x7x_chip_off(indio_dev);
> -
> - return len;
> -}
> -
> static ssize_t in_illuminance0_calibscale_available_show(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> @@ -1494,8 +1467,6 @@ static int tsl2x7x_write_raw(struct iio_dev *indio_dev,
> return 0;
> }
>
> -static DEVICE_ATTR_RW(power_state);
> -
> static DEVICE_ATTR_RO(in_proximity0_calibscale_available);
>
> static DEVICE_ATTR_RO(in_illuminance0_calibscale_available);
> @@ -1580,7 +1551,6 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void
> *private)
> }
>
> static struct attribute *tsl2x7x_ALS_device_attrs[] = {
> - &dev_attr_power_state.attr,
> &dev_attr_in_illuminance0_calibscale_available.attr,
> &dev_attr_in_illuminance0_integration_time.attr,
>
> &iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
> @@ -1591,13 +1561,11 @@ static struct attribute *tsl2x7x_ALS_device_attrs[] =
> {
> };
>
> static struct attribute *tsl2x7x_PRX_device_attrs[] = {
> - &dev_attr_power_state.attr,
> &dev_attr_in_proximity0_calibrate.attr,
> NULL
> };
>
> static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
> - &dev_attr_power_state.attr,
> &dev_attr_in_illuminance0_calibscale_available.attr,
> &dev_attr_in_illuminance0_integration_time.attr,
>
> &iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
> @@ -1609,14 +1577,12 @@ static struct attribute
> *tsl2x7x_ALSPRX_device_attrs[] = {
> };
>
> static struct attribute *tsl2x7x_PRX2_device_attrs[] = {
> - &dev_attr_power_state.attr,
> &dev_attr_in_proximity0_calibrate.attr,
> &dev_attr_in_proximity0_calibscale_available.attr,
> NULL
> };
>
> static struct attribute *tsl2x7x_ALSPRX2_device_attrs[] = {
> - &dev_attr_power_state.attr,
> &dev_attr_in_illuminance0_calibscale_available.attr,
> &dev_attr_in_illuminance0_integration_time.attr,
>
> &iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/9] staging: iio: tsl2x7x: add device tree documentation
On Thu, 6 Jul 2017 18:56:19 -0400 Brian Masney wrote: > Add device tree documentation for the tsl2x7x IIO driver. > > Signed-off-by: Brian Masney > CC: Rob Herring > CC: Mark Rutland > CC: devicet...@vger.kernel.org For what it's worth given the simplicity of the patch Acked-by: Jonathan Cameron > --- > Documentation/devicetree/bindings/trivial-devices.txt | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/Documentation/devicetree/bindings/trivial-devices.txt > b/Documentation/devicetree/bindings/trivial-devices.txt > index 35f406d..0e6e953 100644 > --- a/Documentation/devicetree/bindings/trivial-devices.txt > +++ b/Documentation/devicetree/bindings/trivial-devices.txt > @@ -21,6 +21,16 @@ adi,adt7490+/-1C TDM Extended Temp Range > I.C > adi,adxl345 Three-Axis Digital Accelerometer > adi,adxl346 Three-Axis Digital Accelerometer > (backward-compatibility value "adi,adxl345" must be listed too) > ams,iaq-core AMS iAQ-Core VOC Sensor > +amstaos,tsl2571 AMS/TAOS ALS and proximity sensor > +amstaos,tsl2671 AMS/TAOS ALS and proximity sensor > +amstaos,tmd2671 AMS/TAOS ALS and proximity sensor > +amstaos,tsl2771 AMS/TAOS ALS and proximity sensor > +amstaos,tmd2771 AMS/TAOS ALS and proximity sensor > +amstaos,tsl2572 AMS/TAOS ALS and proximity sensor > +amstaos,tsl2672 AMS/TAOS ALS and proximity sensor > +amstaos,tmd2672 AMS/TAOS ALS and proximity sensor > +amstaos,tsl2772 AMS/TAOS ALS and proximity sensor > +amstaos,tmd2772 AMS/TAOS ALS and proximity sensor > at,24c08 i2c serial eeprom (24cxx) > atmel,at97sc3204ti2c trusted platform module (TPM) > capella,cm32181 CM32181: Ambient Light Sensor ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/9] staging: iio: tsl2x7x: add of_match table for device tree support
On Thu, 6 Jul 2017 18:56:18 -0400
Brian Masney wrote:
> Add device tree support for the tsl2x7x IIO driver with no custom
> properties. The device tree documentation is in a separate commit so
> that the changes to trivial-devices.txt can go in via the device
> tree subsystem.
>
> Signed-off-by: Brian Masney
> CC: Rob Herring
> CC: Mark Rutland
> CC: devicet...@vger.kernel.org
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.
Thanks,
Jonathan
> ---
> drivers/staging/iio/light/tsl2x7x.c | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/staging/iio/light/tsl2x7x.c
> b/drivers/staging/iio/light/tsl2x7x.c
> index 1467199..8148986 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -2026,6 +2026,21 @@ static struct i2c_device_id tsl2x7x_idtable[] = {
>
> MODULE_DEVICE_TABLE(i2c, tsl2x7x_idtable);
>
> +static const struct of_device_id tsl2x7x_of_match[] = {
> + { .compatible = "amstaos,tsl2571" },
> + { .compatible = "amstaos,tsl2671" },
> + { .compatible = "amstaos,tmd2671" },
> + { .compatible = "amstaos,tsl2771" },
> + { .compatible = "amstaos,tmd2771" },
> + { .compatible = "amstaos,tsl2572" },
> + { .compatible = "amstaos,tsl2672" },
> + { .compatible = "amstaos,tmd2672" },
> + { .compatible = "amstaos,tsl2772" },
> + { .compatible = "amstaos,tmd2772" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, tsl2x7x_of_match);
> +
> static const struct dev_pm_ops tsl2x7x_pm_ops = {
> .suspend = tsl2x7x_suspend,
> .resume = tsl2x7x_resume,
> @@ -2035,6 +2050,7 @@ static const struct dev_pm_ops tsl2x7x_pm_ops = {
> static struct i2c_driver tsl2x7x_driver = {
> .driver = {
> .name = "tsl2x7x",
> + .of_match_table = tsl2x7x_of_match,
> .pm = &tsl2x7x_pm_ops,
> },
> .id_table = tsl2x7x_idtable,
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 5/5] Staging: android: use BIT macro
Use BIT macro instead of left shifting in android/ion/ion.h
Signed-off-by: Mateusz Nowotyński
---
drivers/staging/android/ion/ion.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index b91328a..621e5f7 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -135,7 +135,7 @@ struct ion_heap_ops {
/**
* heap flags - flags between the heaps and core ion code
*/
-#define ION_HEAP_FLAG_DEFER_FREE (1 << 0)
+#define ION_HEAP_FLAG_DEFER_FREE BIT(0)
/**
* private flags - flags internal to ion
@@ -146,7 +146,7 @@ struct ion_heap_ops {
* any buffer storage that came from the system allocator will be
* returned to the system allocator.
*/
-#define ION_PRIV_FLAG_SHRINKER_FREE (1 << 0)
+#define ION_PRIV_FLAG_SHRINKER_FREE BIT(0)
/**
* struct ion_heap - represents a heap in the system
--
2.10.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 3/5] Staging: android: Remove unnecessary blank lines
Removes unnecessary blank lines in android/ion/ion_system_heap.c
Signed-off-by: Mateusz Nowotyński
---
drivers/staging/android/ion/ion_system_heap.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/android/ion/ion_system_heap.c
b/drivers/staging/android/ion/ion_system_heap.c
index 5964bf2..4dc5d7a 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -98,7 +98,6 @@ static void free_buffer_page(struct ion_system_heap *heap,
ion_page_pool_free(pool, page);
}
-
static struct page *alloc_largest_available(struct ion_system_heap *heap,
struct ion_buffer *buffer,
unsigned long size,
@@ -256,7 +255,6 @@ static struct ion_heap_ops system_heap_ops = {
static int ion_system_heap_debug_show(struct ion_heap *heap, struct seq_file
*s,
void *unused)
{
-
struct ion_system_heap *sys_heap = container_of(heap,
struct ion_system_heap,
heap);
--
2.10.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 2/5] Staging: android: fix sizeof style issue
Converts sizeof(type) to sizeof(variable) in android/ion/ion_cma_heap.c Signed-off-by: Mateusz Nowotyński --- drivers/staging/android/ion/ion_cma_heap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/android/ion/ion_cma_heap.c b/drivers/staging/android/ion/ion_cma_heap.c index b06f4ce..bb2c144 100644 --- a/drivers/staging/android/ion/ion_cma_heap.c +++ b/drivers/staging/android/ion/ion_cma_heap.c @@ -45,7 +45,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer, if (!pages) return -ENOMEM; - table = kmalloc(sizeof(struct sg_table), GFP_KERNEL); + table = kmalloc(sizeof(*table), GFP_KERNEL); if (!table) goto err; -- 2.10.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 1/5] Staging: android: remove unnecessary blank lines
Removes unnecessary blank lines in android/ion/ion_cma_heap.c
Signed-off-by: Mateusz Nowotyński
---
drivers/staging/android/ion/ion_cma_heap.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/staging/android/ion/ion_cma_heap.c
b/drivers/staging/android/ion/ion_cma_heap.c
index a0949bc..b06f4ce 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -31,7 +31,6 @@ struct ion_cma_heap {
#define to_cma_heap(x) container_of(x, struct ion_cma_heap, heap)
-
/* ION CMA heap operations functions */
static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer,
unsigned long len,
--
2.10.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 4/5] Staging: android: Fix code alignment issue
Fixes code alignment issue in ion/ion.h Signed-off-by: Mateusz Nowotyński --- drivers/staging/android/ion/ion.h | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h index fa9ed81..b91328a 100644 --- a/drivers/staging/android/ion/ion.h +++ b/drivers/staging/android/ion/ion.h @@ -226,8 +226,8 @@ int ion_heap_buffer_zero(struct ion_buffer *buffer); int ion_heap_pages_zero(struct page *page, size_t size, pgprot_t pgprot); int ion_alloc(size_t len, - unsigned int heap_id_mask, - unsigned int flags); + unsigned int heap_id_mask, + unsigned int flags); /** * ion_heap_init_shrinker @@ -291,7 +291,7 @@ size_t ion_heap_freelist_drain(struct ion_heap *heap, size_t size); * flag. */ size_t ion_heap_freelist_shrink(struct ion_heap *heap, - size_t size); + size_t size); /** * ion_heap_freelist_size - returns the size of the freelist in bytes @@ -352,7 +352,7 @@ void ion_page_pool_free(struct ion_page_pool *pool, struct page *page); * returns the number of items freed in pages */ int ion_page_pool_shrink(struct ion_page_pool *pool, gfp_t gfp_mask, - int nr_to_scan); +int nr_to_scan); long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg); -- 2.10.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 1/3] tty: resolve tty contention between kernel and user space
On Sun, Jul 9, 2017 at 2:41 PM, Okash Khawaja wrote:
> +struct tty_struct *tty_kopen(dev_t device)
> +{
> + struct tty_struct *tty;
> + struct tty_driver *driver = NULL;
> + int index = -1;
> +
> + mutex_lock(&tty_mutex);
> + driver = tty_lookup_driver(device, NULL, &index);
> + if (IS_ERR(driver)) {
> + mutex_unlock(&tty_mutex);
> + return ERR_CAST(driver);
Hmm... perhaps
tty = ERR_CAST(driver);
goto out_unlock;
See below for further details.
> + }
> +
> + /* check whether we're reopening an existing tty */
> + tty = tty_driver_lookup_tty(driver, NULL, index);
> + if (IS_ERR(tty))
> + goto out;
> +
> + if (tty) {
> + /* drop kref from tty_driver_lookup_tty() */
> + tty_kref_put(tty);
> + tty = ERR_PTR(-EBUSY);
> + } else { /* tty_init_dev returns tty with the tty_lock held */
> + tty = tty_init_dev(driver, index);
> + set_bit(TTY_KOPENED, &tty->flags);
> + }
> +out:
out_unlock: ?
> + mutex_unlock(&tty_mutex);
> + tty_driver_kref_put(driver);
I'm not sure I understand locking model here:
Above
1. take mutex
2. take reference
Here:
1. give mutex back
2. give reference back
I think we usually see symmetrical calls, i.e.
1. give reference back
2. give mutex back
So, what did I miss?
> + return tty;
> +}
--
With Best Regards,
Andy Shevchenko
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[patch 4/3] tty: make tty_kopen return ENODEV in case of no TTY
When TTY is not built, tty_kopen should return an error. This way
calling code remains consistent, regardless of whether tty is built or
not. This patch returns -ENODEV when there is no tty.
Signed-off-by: Okash Khawaja
---
include/linux/tty.h |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -423,7 +423,7 @@ static inline int __init tty_init(void)
static inline const char *tty_name(const struct tty_struct *tty)
{ return "(none)"; }
static inline struct tty_struct *tty_kopen(dev_t device)
-{ return NULL; }
+{ return ERR_PTR(-ENODEV); }
static inline int tty_dev_name_to_number(const char *name, dev_t *number)
{ return -ENOTSUPP; }
#endif
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 2/3] staging: speakup: use tty_kopen instead of tty_open_by_driver
On Sun, Jul 09, 2017 at 01:50:55PM +0200, Greg Kroah-Hartman wrote: > On Sun, Jul 09, 2017 at 12:41:55PM +0100, Okash Khawaja wrote: > > This patch replaces call to tty_open_by_driver with a tty_kopen. > > > > Signed-off-by: Okash Khawaja > > > > --- > > drivers/staging/speakup/spk_ttyio.c |2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > --- a/drivers/staging/speakup/spk_ttyio.c > > +++ b/drivers/staging/speakup/spk_ttyio.c > > @@ -164,7 +164,7 @@ static int spk_ttyio_initialise_ldisc(st > > if (ret) > > return ret; > > > > - tty = tty_open_by_driver(dev, NULL, NULL); > > + tty = tty_kopen(dev); > > if (IS_ERR(tty)) > > Hm, the "no tty layer" return value for this will be NULL. I doubt > that's really a big deal, but perhaps just have that return > PTR_ERR(-ENODEV) or something? Good point, thanks. Will send a follow up patch Okash ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: atomisp: use kstrdup to replace kmalloc and memcpy
On 8 July 2017 at 16:31, Sakari Ailus wrote:
> Hi Hari,
>
> On Fri, Jul 07, 2017 at 08:15:21PM +0530, Hari Prasath wrote:
>> kstrdup kernel primitive can be used to replace kmalloc followed by
>> string copy. This was reported by coccinelle tool
>>
>> Signed-off-by: Hari Prasath
>> ---
>> .../media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c | 10
>> +++---
>> 1 file changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git
>> a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
>> b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
>> index 34cc56f..68db87b 100644
>> --- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
>> +++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
>> @@ -144,14 +144,10 @@ sh_css_load_blob_info(const char *fw, const struct
>> ia_css_fw_info *bi, struct ia
>> )
>> {
>> char *namebuffer;
>> - int namelength = (int)strlen(name);
>> -
>> - namebuffer = (char *) kmalloc(namelength + 1, GFP_KERNEL);
>> - if (namebuffer == NULL)
>> - return IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY;
>> -
>> - memcpy(namebuffer, name, namelength + 1);
>>
>> + namebuffer = kstrdup(name, GFP_KERNEL);
>> + if (!namebuffer)
>> + return -ENOMEM;
>
> The patch also changes the return value in error cases. I believe the
> caller(s) expect to get errors in the IA_CCS_ERR_* range.
Hi,
In this particular case, the calling function just checks if it's not
success defined by a enum. I think returning -ENOMEM would not effect,
at least in this case.
- Hari Prasath
>
>> bd->name = fw_minibuffer[index].name = namebuffer;
>> } else {
>> bd->name = name;
>
> --
> Regards,
>
> Sakari Ailus
> e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
Regards,
G.E.Hari Prasath
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 0/3] Re: tty contention resulting from tty_open_by_device export
On Sun, Jul 09, 2017 at 12:41:53PM +0100, Okash Khawaja wrote: > On Sat, Jul 08, 2017 at 10:38:03AM +0200, Greg Kroah-Hartman wrote: > > Overall, the idea looks sane to me. Keeping userspace from opening a > > tty that the kernel has opened internally makes sense, hopefully > > userspace doesn't get too confused when that happens. I don't think we > > normally return -EBUSY from an open call, have you seen what happens > > with apps when you do this (like minicom?) > > > I tested this wil minincom, picocom and commands like "echo foo > > /dev/ttyS0". They all correctly report "Device or resource busy". > > I have addressed all the comments you made. I have also split the patch > into three. Following is summary of each. > > Patch 1: introduces the tty_kopen function and checks for TTY_KOPENED > Patch 2: updates speakup code to use tty_kopen instead of > tty_open_by_driver > Patch 3: reverses the export of tty_open_by_driver Looks great, only one tiny comment about the return value from me, and really, it's not a big deal at all, you can send a patch 4/3 to change that up if you want. No one really runs without the tty layer enabled :) If there are no other objections, I'll queue this up after 4.13-rc1 is out, nice job. greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 1/3] tty: resolve tty contention between kernel and user space
On Sun, Jul 09, 2017 at 12:41:54PM +0100, Okash Khawaja wrote:
> The commit 12e84c71b7d4ee (tty: export tty_open_by_driver) exports
> tty_open_by_device to allow tty to be opened from inside kernel which
> works fine except that it doesn't handle contention with user space or
> another kernel-space open of the same tty. For example, opening a tty
> from user space while it is kernel opened results in failure and a
> kernel log message about mismatch between tty->count and tty's file
> open count.
>
> This patch makes kernel access to tty exclusive, so that if a user
> process or kernel opens a kernel opened tty, it gets -EBUSY. It does
> this by adding TTY_KOPENED flag to tty->flags. When this flag is set,
> tty_open_by_driver returns -EBUSY. Instead of overlaoding
> tty_open_by_driver for both kernel and user space, this
> patch creates a separate function tty_kopen which closely follows
> tty_open_by_driver.
>
> Returning -EBUSY on tty open is a change in the interface. I have
> tested this with minicom, picocom and commands like "echo foo >
> /dev/ttyS0". They all correctly report "Device or resource busy" when
> the tty is already kernel opened.
>
> Signed-off-by: Okash Khawaja
>
> ---
> drivers/tty/tty_io.c | 54
> +++
> include/linux/tty.h |4 +++
> 2 files changed, 58 insertions(+)
>
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -1786,6 +1786,54 @@ static struct tty_driver *tty_lookup_dri
> }
>
> /**
> + * tty_kopen - open a tty device for kernel
> + * @device: dev_t of device to open
> + *
> + * Opens tty exclusively for kernel. Performs the driver lookup,
> + * makes sure it's not already opened and performs the first-time
> + * tty initialization.
> + *
> + * Returns the locked initialized &tty_struct
> + *
> + * Claims the global tty_mutex to serialize:
> + * - concurrent first-time tty initialization
> + * - concurrent tty driver removal w/ lookup
> + * - concurrent tty removal from driver table
> + */
> +struct tty_struct *tty_kopen(dev_t device)
> +{
> + struct tty_struct *tty;
> + struct tty_driver *driver = NULL;
> + int index = -1;
> +
> + mutex_lock(&tty_mutex);
> + driver = tty_lookup_driver(device, NULL, &index);
> + if (IS_ERR(driver)) {
> + mutex_unlock(&tty_mutex);
> + return ERR_CAST(driver);
> + }
> +
> + /* check whether we're reopening an existing tty */
> + tty = tty_driver_lookup_tty(driver, NULL, index);
> + if (IS_ERR(tty))
> + goto out;
> +
> + if (tty) {
> + /* drop kref from tty_driver_lookup_tty() */
> + tty_kref_put(tty);
> + tty = ERR_PTR(-EBUSY);
> + } else { /* tty_init_dev returns tty with the tty_lock held */
> + tty = tty_init_dev(driver, index);
> + set_bit(TTY_KOPENED, &tty->flags);
> + }
> +out:
> + mutex_unlock(&tty_mutex);
> + tty_driver_kref_put(driver);
> + return tty;
> +}
> +EXPORT_SYMBOL_GPL(tty_kopen);
> +
> +/**
> * tty_open_by_driver - open a tty device
> * @device: dev_t of device to open
> * @inode: inode of device file
> @@ -1824,6 +1872,12 @@ struct tty_struct *tty_open_by_driver(de
> }
>
> if (tty) {
> + if (test_bit(TTY_KOPENED, &tty->flags)) {
> + tty_kref_put(tty);
> + mutex_unlock(&tty_mutex);
> + tty = ERR_PTR(-EBUSY);
> + goto out;
> + }
> mutex_unlock(&tty_mutex);
> retval = tty_lock_interruptible(tty);
> tty_kref_put(tty); /* drop kref from tty_driver_lookup_tty() */
> --- a/include/linux/tty.h
> +++ b/include/linux/tty.h
> @@ -362,6 +362,7 @@ struct tty_file_private {
> #define TTY_NO_WRITE_SPLIT 17 /* Preserve write boundaries to driver
> */
> #define TTY_HUPPED 18 /* Post driver->hangup() */
> #define TTY_LDISC_HALTED 22 /* Line discipline is halted */
> +#define TTY_KOPENED 23 /* TTY exclusively opened by kernel */
>
> /* Values for tty->flow_change */
> #define TTY_THROTTLE_SAFE 1
> @@ -401,6 +402,7 @@ extern int __init tty_init(void);
> extern const char *tty_name(const struct tty_struct *tty);
> extern struct tty_struct *tty_open_by_driver(dev_t device, struct inode
> *inode,
> struct file *filp);
> +extern struct tty_struct *tty_kopen(dev_t device);
> extern int tty_dev_name_to_number(const char *name, dev_t *number);
> #else
> static inline void tty_kref_put(struct tty_struct *tty)
> @@ -425,6 +427,8 @@ static inline const char *tty_name(const
> static inline struct tty_struct *tty_open_by_driver(dev_t device,
> struct inode *inode, struct file *filp)
> { return NULL; }
> +static inline struct tty_struct *tty_kopen(dev_t device)
> +{ return NULL; }
Like I said in response to patch 2, m
Re: [patch 2/3] staging: speakup: use tty_kopen instead of tty_open_by_driver
On Sun, Jul 09, 2017 at 12:41:55PM +0100, Okash Khawaja wrote: > This patch replaces call to tty_open_by_driver with a tty_kopen. > > Signed-off-by: Okash Khawaja > > --- > drivers/staging/speakup/spk_ttyio.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > --- a/drivers/staging/speakup/spk_ttyio.c > +++ b/drivers/staging/speakup/spk_ttyio.c > @@ -164,7 +164,7 @@ static int spk_ttyio_initialise_ldisc(st > if (ret) > return ret; > > - tty = tty_open_by_driver(dev, NULL, NULL); > + tty = tty_kopen(dev); > if (IS_ERR(tty)) Hm, the "no tty layer" return value for this will be NULL. I doubt that's really a big deal, but perhaps just have that return PTR_ERR(-ENODEV) or something? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[patch 3/3] tty: undo export of tty_open_by_driver
Since we have tty_kopen, we no longer need to export tty_open_by_driver.
This patch makes this function static.
Signed-off-by: Okash Khawaja
---
drivers/tty/tty_io.c |3 +--
include/linux/tty.h |5 -
2 files changed, 1 insertion(+), 7 deletions(-)
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1849,7 +1849,7 @@ EXPORT_SYMBOL_GPL(tty_kopen);
* - concurrent tty driver removal w/ lookup
* - concurrent tty removal from driver table
*/
-struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
+static struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
struct file *filp)
{
struct tty_struct *tty;
@@ -1900,7 +1900,6 @@ out:
tty_driver_kref_put(driver);
return tty;
}
-EXPORT_SYMBOL_GPL(tty_open_by_driver);
/**
* tty_open- open a tty device
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -400,8 +400,6 @@ extern struct tty_struct *get_current_tt
/* tty_io.c */
extern int __init tty_init(void);
extern const char *tty_name(const struct tty_struct *tty);
-extern struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
- struct file *filp);
extern struct tty_struct *tty_kopen(dev_t device);
extern int tty_dev_name_to_number(const char *name, dev_t *number);
#else
@@ -424,9 +422,6 @@ static inline int __init tty_init(void)
{ return 0; }
static inline const char *tty_name(const struct tty_struct *tty)
{ return "(none)"; }
-static inline struct tty_struct *tty_open_by_driver(dev_t device,
- struct inode *inode, struct file *filp)
-{ return NULL; }
static inline struct tty_struct *tty_kopen(dev_t device)
{ return NULL; }
static inline int tty_dev_name_to_number(const char *name, dev_t *number)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[patch 0/3] Re: tty contention resulting from tty_open_by_device export
On Sat, Jul 08, 2017 at 10:38:03AM +0200, Greg Kroah-Hartman wrote: > Overall, the idea looks sane to me. Keeping userspace from opening a > tty that the kernel has opened internally makes sense, hopefully > userspace doesn't get too confused when that happens. I don't think we > normally return -EBUSY from an open call, have you seen what happens > with apps when you do this (like minicom?) > I tested this wil minincom, picocom and commands like "echo foo > /dev/ttyS0". They all correctly report "Device or resource busy". I have addressed all the comments you made. I have also split the patch into three. Following is summary of each. Patch 1: introduces the tty_kopen function and checks for TTY_KOPENED Patch 2: updates speakup code to use tty_kopen instead of tty_open_by_driver Patch 3: reverses the export of tty_open_by_driver Thanks, Okash ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[patch 2/3] staging: speakup: use tty_kopen instead of tty_open_by_driver
This patch replaces call to tty_open_by_driver with a tty_kopen. Signed-off-by: Okash Khawaja --- drivers/staging/speakup/spk_ttyio.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- a/drivers/staging/speakup/spk_ttyio.c +++ b/drivers/staging/speakup/spk_ttyio.c @@ -164,7 +164,7 @@ static int spk_ttyio_initialise_ldisc(st if (ret) return ret; - tty = tty_open_by_driver(dev, NULL, NULL); + tty = tty_kopen(dev); if (IS_ERR(tty)) return PTR_ERR(tty); ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[patch 1/3] tty: resolve tty contention between kernel and user space
The commit 12e84c71b7d4ee (tty: export tty_open_by_driver) exports
tty_open_by_device to allow tty to be opened from inside kernel which
works fine except that it doesn't handle contention with user space or
another kernel-space open of the same tty. For example, opening a tty
from user space while it is kernel opened results in failure and a
kernel log message about mismatch between tty->count and tty's file
open count.
This patch makes kernel access to tty exclusive, so that if a user
process or kernel opens a kernel opened tty, it gets -EBUSY. It does
this by adding TTY_KOPENED flag to tty->flags. When this flag is set,
tty_open_by_driver returns -EBUSY. Instead of overlaoding
tty_open_by_driver for both kernel and user space, this
patch creates a separate function tty_kopen which closely follows
tty_open_by_driver.
Returning -EBUSY on tty open is a change in the interface. I have
tested this with minicom, picocom and commands like "echo foo >
/dev/ttyS0". They all correctly report "Device or resource busy" when
the tty is already kernel opened.
Signed-off-by: Okash Khawaja
---
drivers/tty/tty_io.c | 54 +++
include/linux/tty.h |4 +++
2 files changed, 58 insertions(+)
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1786,6 +1786,54 @@ static struct tty_driver *tty_lookup_dri
}
/**
+ * tty_kopen - open a tty device for kernel
+ * @device: dev_t of device to open
+ *
+ * Opens tty exclusively for kernel. Performs the driver lookup,
+ * makes sure it's not already opened and performs the first-time
+ * tty initialization.
+ *
+ * Returns the locked initialized &tty_struct
+ *
+ * Claims the global tty_mutex to serialize:
+ * - concurrent first-time tty initialization
+ * - concurrent tty driver removal w/ lookup
+ * - concurrent tty removal from driver table
+ */
+struct tty_struct *tty_kopen(dev_t device)
+{
+ struct tty_struct *tty;
+ struct tty_driver *driver = NULL;
+ int index = -1;
+
+ mutex_lock(&tty_mutex);
+ driver = tty_lookup_driver(device, NULL, &index);
+ if (IS_ERR(driver)) {
+ mutex_unlock(&tty_mutex);
+ return ERR_CAST(driver);
+ }
+
+ /* check whether we're reopening an existing tty */
+ tty = tty_driver_lookup_tty(driver, NULL, index);
+ if (IS_ERR(tty))
+ goto out;
+
+ if (tty) {
+ /* drop kref from tty_driver_lookup_tty() */
+ tty_kref_put(tty);
+ tty = ERR_PTR(-EBUSY);
+ } else { /* tty_init_dev returns tty with the tty_lock held */
+ tty = tty_init_dev(driver, index);
+ set_bit(TTY_KOPENED, &tty->flags);
+ }
+out:
+ mutex_unlock(&tty_mutex);
+ tty_driver_kref_put(driver);
+ return tty;
+}
+EXPORT_SYMBOL_GPL(tty_kopen);
+
+/**
* tty_open_by_driver - open a tty device
* @device: dev_t of device to open
* @inode: inode of device file
@@ -1824,6 +1872,12 @@ struct tty_struct *tty_open_by_driver(de
}
if (tty) {
+ if (test_bit(TTY_KOPENED, &tty->flags)) {
+ tty_kref_put(tty);
+ mutex_unlock(&tty_mutex);
+ tty = ERR_PTR(-EBUSY);
+ goto out;
+ }
mutex_unlock(&tty_mutex);
retval = tty_lock_interruptible(tty);
tty_kref_put(tty); /* drop kref from tty_driver_lookup_tty() */
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -362,6 +362,7 @@ struct tty_file_private {
#define TTY_NO_WRITE_SPLIT 17 /* Preserve write boundaries to driver
*/
#define TTY_HUPPED 18 /* Post driver->hangup() */
#define TTY_LDISC_HALTED 22 /* Line discipline is halted */
+#define TTY_KOPENED23 /* TTY exclusively opened by kernel */
/* Values for tty->flow_change */
#define TTY_THROTTLE_SAFE 1
@@ -401,6 +402,7 @@ extern int __init tty_init(void);
extern const char *tty_name(const struct tty_struct *tty);
extern struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
struct file *filp);
+extern struct tty_struct *tty_kopen(dev_t device);
extern int tty_dev_name_to_number(const char *name, dev_t *number);
#else
static inline void tty_kref_put(struct tty_struct *tty)
@@ -425,6 +427,8 @@ static inline const char *tty_name(const
static inline struct tty_struct *tty_open_by_driver(dev_t device,
struct inode *inode, struct file *filp)
{ return NULL; }
+static inline struct tty_struct *tty_kopen(dev_t device)
+{ return NULL; }
static inline int tty_dev_name_to_number(const char *name, dev_t *number)
{ return -ENOTSUPP; }
#endif
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/ma
