RE: [PATCH 4/5] Drivers: hv: kvp: Use %u to print U32
> -Original Message- > From: Greg KH > Sent: Tuesday, October 16, 2018 10:04 PM > To: KY Srinivasan > Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org; > o...@aepfle.de; a...@canonical.com; jasow...@redhat.com; Stephen > Hemminger ; Michael Kelley (EOSG) > ; vkuznets ; > Haiyang Zhang ; sta...@vger.kernel.org > Subject: Re: [PATCH 4/5] Drivers: hv: kvp: Use %u to print U32 > > On Wed, Oct 17, 2018 at 03:14:05AM +, k...@linuxonhyperv.com wrote: > > From: Dexuan Cui > > > > I didn't find a real issue. Let's just make it consistent with the > > next "case REG_U64:" where %llu is used. > > > > Signed-off-by: Dexuan Cui > > Cc: K. Y. Srinivasan > > Cc: Haiyang Zhang > > Cc: Stephen Hemminger > > Cc: > > Why is this a stable patch? My mistake; I will resend. K. Y > > confused, > > greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 1/5] Drivers: hv: vmbus: Get rid of unnecessary state in hv_context
> -Original Message- > From: Greg KH > Sent: Tuesday, October 16, 2018 10:05 PM > To: KY Srinivasan > Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org; > o...@aepfle.de; a...@canonical.com; jasow...@redhat.com; Stephen > Hemminger ; Michael Kelley (EOSG) > ; vkuznets > Subject: Re: [PATCH 1/5] Drivers: hv: vmbus: Get rid of unnecessary state in > hv_context > > On Wed, Oct 17, 2018 at 03:14:02AM +, k...@linuxonhyperv.com wrote: > > From: "K. Y. Srinivasan" > > > > Currently we are replicating state in struct hv_context that is unnecessary > > - > > this state can be retrieved from the hypervisor. Furthermore, this is a per- > cpu > > state that is being maintained as a global state in struct hv_context. > > Get rid of this state in struct hv_context. > > > > Reply-To: k...@microsoft.com > > Why is this here? No sure how this happened; will fix it and resend. K. Y > > greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/5] Drivers: hv: kvp: Fix the recent regression caused by incorrect clean-up
On 10/17/18 7:07 AM, Greg KH wrote:
> On Wed, Oct 17, 2018 at 03:14:04AM +, k...@linuxonhyperv.com wrote:
>> From: Dexuan Cui
>>
>> In kvp_send_key(), we do need call process_ib_ipinfo() if
>> message->kvp_hdr.operation is KVP_OP_GET_IP_INFO, because it turns out
>> the userland hv_kvp_daemon needs the info of operation, adapter_id and
>> addr_family. With the incorrect fc62c3b1977d, the host can't get the
>> VM's IP via KVP.
>>
>> And, fc62c3b1977d added a "break;", but actually forgot to initialize
>> the key_size/value in the case of KVP_OP_SET, so the default key_size of
>> 0 is passed to the kvp daemon, and the pool files
>> /var/lib/hyperv/.kvp_pool_* can't be updated.
>>
>> This patch effectively rolls back the previous fc62c3b1977d, and
>> correctly fixes the "this statement may fall through" warnings.
>>
>> This patch is tested on WS 2012 R2 and 2016.
>>
>> Fixes: fc62c3b1977d ("Drivers: hv: kvp: Fix two "this statement may fall
>> through" warnings")
>> Signed-off-by: Dexuan Cui
>> Cc: K. Y. Srinivasan
>> Cc: Haiyang Zhang
>> Cc: Stephen Hemminger
>> Cc:
>> Signed-off-by: K. Y. Srinivasan
>> ---
>> drivers/hv/hv_kvp.c | 26 ++
>> 1 file changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
>> index a7513a8a8e37..9fbb15c62c6c 100644
>> --- a/drivers/hv/hv_kvp.c
>> +++ b/drivers/hv/hv_kvp.c
>> @@ -353,6 +353,9 @@ static void process_ib_ipinfo(void *in_msg, void
>> *out_msg, int op)
>>
>> out->body.kvp_ip_val.dhcp_enabled = in->kvp_ip_val.dhcp_enabled;
>>
>> +__attribute__ ((fallthrough));
>
> The comment should be sufficient for this, right? I haven't seen many
> uses of this attribute before, how common is it?
>
It's not common at all.
Dexuan, please use a comment instead: /* fall through */
Thanks
--
Gustavo
>
>> +
>> +case KVP_OP_GET_IP_INFO:
>> utf16s_to_utf8s((wchar_t *)in->kvp_ip_val.adapter_id,
>> MAX_ADAPTER_ID_SIZE,
>> UTF16_LITTLE_ENDIAN,
>> @@ -405,7 +408,11 @@ kvp_send_key(struct work_struct *dummy)
>> process_ib_ipinfo(in_msg, message, KVP_OP_SET_IP_INFO);
>> break;
>> case KVP_OP_GET_IP_INFO:
>> -/* We only need to pass on message->kvp_hdr.operation. */
>> +/*
>> + * We only need to pass on the info of operation, adapter_id
>> + * and addr_family to the userland kvp daemon.
>> + */
>> +process_ib_ipinfo(in_msg, message, KVP_OP_GET_IP_INFO);
>> break;
>> case KVP_OP_SET:
>> switch (in_msg->body.kvp_set.data.value_type) {
>> @@ -446,9 +453,9 @@ kvp_send_key(struct work_struct *dummy)
>>
>> }
>>
>> -break;
>> -
>> -case KVP_OP_GET:
>> +/*
>> + * The key is always a string - utf16 encoding.
>> + */
>> message->body.kvp_set.data.key_size =
>> utf16s_to_utf8s(
>> (wchar_t *)in_msg->body.kvp_set.data.key,
>> @@ -456,6 +463,17 @@ kvp_send_key(struct work_struct *dummy)
>> UTF16_LITTLE_ENDIAN,
>> message->body.kvp_set.data.key,
>> HV_KVP_EXCHANGE_MAX_KEY_SIZE - 1) + 1;
>> +
>> +break;
>> +
>> +case KVP_OP_GET:
>> +message->body.kvp_get.data.key_size =
>> +utf16s_to_utf8s(
>> +(wchar_t *)in_msg->body.kvp_get.data.key,
>> +in_msg->body.kvp_get.data.key_size,
>> +UTF16_LITTLE_ENDIAN,
>> +message->body.kvp_get.data.key,
>> +HV_KVP_EXCHANGE_MAX_KEY_SIZE - 1) + 1;
>
> Worst indentation ever :(
>
> Yeah, I know it follows the others above it, but you should reconsider
> it in the future...
>
> thanks,
>
> greg k-h
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/5] Drivers: hv: kvp: Fix the recent regression caused by incorrect clean-up
On Wed, Oct 17, 2018 at 03:14:04AM +, k...@linuxonhyperv.com wrote:
> From: Dexuan Cui
>
> In kvp_send_key(), we do need call process_ib_ipinfo() if
> message->kvp_hdr.operation is KVP_OP_GET_IP_INFO, because it turns out
> the userland hv_kvp_daemon needs the info of operation, adapter_id and
> addr_family. With the incorrect fc62c3b1977d, the host can't get the
> VM's IP via KVP.
>
> And, fc62c3b1977d added a "break;", but actually forgot to initialize
> the key_size/value in the case of KVP_OP_SET, so the default key_size of
> 0 is passed to the kvp daemon, and the pool files
> /var/lib/hyperv/.kvp_pool_* can't be updated.
>
> This patch effectively rolls back the previous fc62c3b1977d, and
> correctly fixes the "this statement may fall through" warnings.
>
> This patch is tested on WS 2012 R2 and 2016.
>
> Fixes: fc62c3b1977d ("Drivers: hv: kvp: Fix two "this statement may fall
> through" warnings")
> Signed-off-by: Dexuan Cui
> Cc: K. Y. Srinivasan
> Cc: Haiyang Zhang
> Cc: Stephen Hemminger
> Cc:
> Signed-off-by: K. Y. Srinivasan
> ---
> drivers/hv/hv_kvp.c | 26 ++
> 1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
> index a7513a8a8e37..9fbb15c62c6c 100644
> --- a/drivers/hv/hv_kvp.c
> +++ b/drivers/hv/hv_kvp.c
> @@ -353,6 +353,9 @@ static void process_ib_ipinfo(void *in_msg, void
> *out_msg, int op)
>
> out->body.kvp_ip_val.dhcp_enabled = in->kvp_ip_val.dhcp_enabled;
>
> + __attribute__ ((fallthrough));
The comment should be sufficient for this, right? I haven't seen many
uses of this attribute before, how common is it?
> +
> + case KVP_OP_GET_IP_INFO:
> utf16s_to_utf8s((wchar_t *)in->kvp_ip_val.adapter_id,
> MAX_ADAPTER_ID_SIZE,
> UTF16_LITTLE_ENDIAN,
> @@ -405,7 +408,11 @@ kvp_send_key(struct work_struct *dummy)
> process_ib_ipinfo(in_msg, message, KVP_OP_SET_IP_INFO);
> break;
> case KVP_OP_GET_IP_INFO:
> - /* We only need to pass on message->kvp_hdr.operation. */
> + /*
> + * We only need to pass on the info of operation, adapter_id
> + * and addr_family to the userland kvp daemon.
> + */
> + process_ib_ipinfo(in_msg, message, KVP_OP_GET_IP_INFO);
> break;
> case KVP_OP_SET:
> switch (in_msg->body.kvp_set.data.value_type) {
> @@ -446,9 +453,9 @@ kvp_send_key(struct work_struct *dummy)
>
> }
>
> - break;
> -
> - case KVP_OP_GET:
> + /*
> + * The key is always a string - utf16 encoding.
> + */
> message->body.kvp_set.data.key_size =
> utf16s_to_utf8s(
> (wchar_t *)in_msg->body.kvp_set.data.key,
> @@ -456,6 +463,17 @@ kvp_send_key(struct work_struct *dummy)
> UTF16_LITTLE_ENDIAN,
> message->body.kvp_set.data.key,
> HV_KVP_EXCHANGE_MAX_KEY_SIZE - 1) + 1;
> +
> + break;
> +
> + case KVP_OP_GET:
> + message->body.kvp_get.data.key_size =
> + utf16s_to_utf8s(
> + (wchar_t *)in_msg->body.kvp_get.data.key,
> + in_msg->body.kvp_get.data.key_size,
> + UTF16_LITTLE_ENDIAN,
> + message->body.kvp_get.data.key,
> + HV_KVP_EXCHANGE_MAX_KEY_SIZE - 1) + 1;
Worst indentation ever :(
Yeah, I know it follows the others above it, but you should reconsider
it in the future...
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/5] Tools: hv: kvp: Fix a warning of buffer overflow with gcc 8.0.1
On Wed, Oct 17, 2018 at 03:14:06AM +, k...@linuxonhyperv.com wrote: > From: Dexuan Cui > > The patch fixes: > > hv_kvp_daemon.c: In function 'kvp_set_ip_info': > hv_kvp_daemon.c:1305:2: note: 'snprintf' output between 41 and 4136 bytes > into a destination of size 4096 > > Signed-off-by: Dexuan Cui > Cc: K. Y. Srinivasan > Cc: Haiyang Zhang > Cc: Stephen Hemminger > Cc: > Signed-off-by: K. Y. Srinivasan > --- > tools/hv/hv_kvp_daemon.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c > index bbb2a8ef367c..b7c2cf7eaba5 100644 > --- a/tools/hv/hv_kvp_daemon.c > +++ b/tools/hv/hv_kvp_daemon.c > @@ -1176,7 +1176,7 @@ static int kvp_set_ip_info(char *if_name, struct > hv_kvp_ipaddr_value *new_val) > int error = 0; > char if_file[PATH_MAX]; > FILE *file; > - char cmd[PATH_MAX]; > + char cmd[PATH_MAX * 2]; Overkill? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/5] Drivers: hv: vmbus: Get rid of unnecessary state in hv_context
On Wed, Oct 17, 2018 at 03:14:02AM +, k...@linuxonhyperv.com wrote: > From: "K. Y. Srinivasan" > > Currently we are replicating state in struct hv_context that is unnecessary - > this state can be retrieved from the hypervisor. Furthermore, this is a > per-cpu > state that is being maintained as a global state in struct hv_context. > Get rid of this state in struct hv_context. > > Reply-To: k...@microsoft.com Why is this here? greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/5] Drivers: hv: kvp: Use %u to print U32
On Wed, Oct 17, 2018 at 03:14:05AM +, k...@linuxonhyperv.com wrote: > From: Dexuan Cui > > I didn't find a real issue. Let's just make it consistent with the > next "case REG_U64:" where %llu is used. > > Signed-off-by: Dexuan Cui > Cc: K. Y. Srinivasan > Cc: Haiyang Zhang > Cc: Stephen Hemminger > Cc: Why is this a stable patch? confused, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/5] Drivers: hv: vmbus: Get rid of unnecessary state in hv_context
From: "K. Y. Srinivasan"
Currently we are replicating state in struct hv_context that is unnecessary -
this state can be retrieved from the hypervisor. Furthermore, this is a per-cpu
state that is being maintained as a global state in struct hv_context.
Get rid of this state in struct hv_context.
Reply-To: k...@microsoft.com
Signed-off-by: K. Y. Srinivasan
---
drivers/hv/hv.c | 10 +++---
drivers/hv/hyperv_vmbus.h | 2 --
2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 332d7c34be5c..166c2501de17 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -33,9 +33,7 @@
#include "hyperv_vmbus.h"
/* The one and only */
-struct hv_context hv_context = {
- .synic_initialized = false,
-};
+struct hv_context hv_context;
/*
* If false, we're using the old mechanism for stimer0 interrupts
@@ -326,8 +324,6 @@ int hv_synic_init(unsigned int cpu)
hv_set_synic_state(sctrl.as_uint64);
- hv_context.synic_initialized = true;
-
/*
* Register the per-cpu clockevent source.
*/
@@ -373,7 +369,8 @@ int hv_synic_cleanup(unsigned int cpu)
bool channel_found = false;
unsigned long flags;
- if (!hv_context.synic_initialized)
+ hv_get_synic_state(sctrl.as_uint64);
+ if (sctrl.enable != 1)
return -EFAULT;
/*
@@ -435,7 +432,6 @@ int hv_synic_cleanup(unsigned int cpu)
hv_set_siefp(siefp.as_uint64);
/* Disable the global synic bit */
- hv_get_synic_state(sctrl.as_uint64);
sctrl.enable = 0;
hv_set_synic_state(sctrl.as_uint64);
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 72eaba3d50fc..f17c06a5e74b 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -230,8 +230,6 @@ struct hv_context {
void *tsc_page;
- bool synic_initialized;
-
struct hv_per_cpu_context __percpu *cpu_context;
/*
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/5] Drivers: hv: kvp: Use %u to print U32
From: Dexuan Cui I didn't find a real issue. Let's just make it consistent with the next "case REG_U64:" where %llu is used. Signed-off-by: Dexuan Cui Cc: K. Y. Srinivasan Cc: Haiyang Zhang Cc: Stephen Hemminger Cc: Signed-off-by: K. Y. Srinivasan --- drivers/hv/hv_kvp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c index 9fbb15c62c6c..3b8590ff94ba 100644 --- a/drivers/hv/hv_kvp.c +++ b/drivers/hv/hv_kvp.c @@ -437,7 +437,7 @@ kvp_send_key(struct work_struct *dummy) val32 = in_msg->body.kvp_set.data.value_u32; message->body.kvp_set.data.value_size = sprintf(message->body.kvp_set.data.value, - "%d", val32) + 1; + "%u", val32) + 1; break; case REG_U64: -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/5] Tools: hv: kvp: Fix a warning of buffer overflow with gcc 8.0.1
From: Dexuan Cui The patch fixes: hv_kvp_daemon.c: In function 'kvp_set_ip_info': hv_kvp_daemon.c:1305:2: note: 'snprintf' output between 41 and 4136 bytes into a destination of size 4096 Signed-off-by: Dexuan Cui Cc: K. Y. Srinivasan Cc: Haiyang Zhang Cc: Stephen Hemminger Cc: Signed-off-by: K. Y. Srinivasan --- tools/hv/hv_kvp_daemon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c index bbb2a8ef367c..b7c2cf7eaba5 100644 --- a/tools/hv/hv_kvp_daemon.c +++ b/tools/hv/hv_kvp_daemon.c @@ -1176,7 +1176,7 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val) int error = 0; char if_file[PATH_MAX]; FILE *file; - char cmd[PATH_MAX]; + char cmd[PATH_MAX * 2]; char *mac_addr; /* -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/5] Drivers: hv: kvp: Fix the recent regression caused by incorrect clean-up
From: Dexuan Cui
In kvp_send_key(), we do need call process_ib_ipinfo() if
message->kvp_hdr.operation is KVP_OP_GET_IP_INFO, because it turns out
the userland hv_kvp_daemon needs the info of operation, adapter_id and
addr_family. With the incorrect fc62c3b1977d, the host can't get the
VM's IP via KVP.
And, fc62c3b1977d added a "break;", but actually forgot to initialize
the key_size/value in the case of KVP_OP_SET, so the default key_size of
0 is passed to the kvp daemon, and the pool files
/var/lib/hyperv/.kvp_pool_* can't be updated.
This patch effectively rolls back the previous fc62c3b1977d, and
correctly fixes the "this statement may fall through" warnings.
This patch is tested on WS 2012 R2 and 2016.
Fixes: fc62c3b1977d ("Drivers: hv: kvp: Fix two "this statement may fall
through" warnings")
Signed-off-by: Dexuan Cui
Cc: K. Y. Srinivasan
Cc: Haiyang Zhang
Cc: Stephen Hemminger
Cc:
Signed-off-by: K. Y. Srinivasan
---
drivers/hv/hv_kvp.c | 26 ++
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index a7513a8a8e37..9fbb15c62c6c 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -353,6 +353,9 @@ static void process_ib_ipinfo(void *in_msg, void *out_msg,
int op)
out->body.kvp_ip_val.dhcp_enabled = in->kvp_ip_val.dhcp_enabled;
+ __attribute__ ((fallthrough));
+
+ case KVP_OP_GET_IP_INFO:
utf16s_to_utf8s((wchar_t *)in->kvp_ip_val.adapter_id,
MAX_ADAPTER_ID_SIZE,
UTF16_LITTLE_ENDIAN,
@@ -405,7 +408,11 @@ kvp_send_key(struct work_struct *dummy)
process_ib_ipinfo(in_msg, message, KVP_OP_SET_IP_INFO);
break;
case KVP_OP_GET_IP_INFO:
- /* We only need to pass on message->kvp_hdr.operation. */
+ /*
+* We only need to pass on the info of operation, adapter_id
+* and addr_family to the userland kvp daemon.
+*/
+ process_ib_ipinfo(in_msg, message, KVP_OP_GET_IP_INFO);
break;
case KVP_OP_SET:
switch (in_msg->body.kvp_set.data.value_type) {
@@ -446,9 +453,9 @@ kvp_send_key(struct work_struct *dummy)
}
- break;
-
- case KVP_OP_GET:
+ /*
+* The key is always a string - utf16 encoding.
+*/
message->body.kvp_set.data.key_size =
utf16s_to_utf8s(
(wchar_t *)in_msg->body.kvp_set.data.key,
@@ -456,6 +463,17 @@ kvp_send_key(struct work_struct *dummy)
UTF16_LITTLE_ENDIAN,
message->body.kvp_set.data.key,
HV_KVP_EXCHANGE_MAX_KEY_SIZE - 1) + 1;
+
+ break;
+
+ case KVP_OP_GET:
+ message->body.kvp_get.data.key_size =
+ utf16s_to_utf8s(
+ (wchar_t *)in_msg->body.kvp_get.data.key,
+ in_msg->body.kvp_get.data.key_size,
+ UTF16_LITTLE_ENDIAN,
+ message->body.kvp_get.data.key,
+ HV_KVP_EXCHANGE_MAX_KEY_SIZE - 1) + 1;
break;
case KVP_OP_DELETE:
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/5] hv_utils: update name in struct hv_driver util_drv
From: Haiyang Zhang
The correct module name is hv_utils. This patch corrects
the name in struct hv_driver util_drv.
Signed-off-by: Haiyang Zhang
Signed-off-by: K. Y. Srinivasan
---
drivers/hv/hv_util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 423205077bf6..f10eeb120c8b 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -483,7 +483,7 @@ MODULE_DEVICE_TABLE(vmbus, id_table);
/* The one and only one */
static struct hv_driver util_drv = {
- .name = "hv_util",
+ .name = "hv_utils",
.id_table = id_table,
.probe = util_probe,
.remove = util_remove,
--
2.18.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/5] Drivers: hv: Miscellaneous fixes
From: "K. Y. Srinivasan" Miscellaneous fixes. Dexuan Cui (3): Drivers: hv: kvp: Fix the recent regression caused by incorrect clean-up Drivers: hv: kvp: Use %u to print U32 Tools: hv: kvp: Fix a warning of buffer overflow with gcc 8.0.1 Haiyang Zhang (1): hv_utils: update name in struct hv_driver util_drv K. Y. Srinivasan (1): Drivers: hv: vmbus: Get rid of unnecessary state in hv_context drivers/hv/hv.c | 10 +++--- drivers/hv/hv_kvp.c | 28 +++- drivers/hv/hv_util.c | 2 +- drivers/hv/hyperv_vmbus.h | 2 -- tools/hv/hv_kvp_daemon.c | 2 +- 5 files changed, 28 insertions(+), 16 deletions(-) -- 2.18.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: ad7280a: Lines should not end with a '(' - style
On Tue, 2018-10-16 at 20:29 -0300, Giuliano Augusto Faulin Belinassi wrote: > > (There is a linux-...@googlegroups.com mailing list > > that bounces when I reply, so I removed it from the > > cc list) > > Sorry. I think this may be because my HTML mode in gmail was enabled. No, it is because that group address is private and rejects posts from non-members. > > Are you doing this for a class assignment? > Yes, I am. I am into a group that aims to contribute to opensource > projects and we chose the Linux kernel. Our mentor suggested us to > contribute to the linux-iio That's fine but please tell your mentor to try to vet the proposed patches within your internal groups before sending them on to lkml. Perhaps add the kernel-newbies list to the vetting. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 10/12] media: imx-csi: Move crop/compose reset after filling default mbus fields
If caller passes un-initialized field type V4L2_FIELD_ANY to CSI
sink pad, the reset CSI crop window would not be correct, because
the crop window depends on a valid input field type. To fix move
the reset of crop and compose windows to after the call to
imx_media_fill_default_mbus_fields().
Signed-off-by: Steve Longerbeam
Reviewed-by: Philipp Zabel
---
drivers/staging/media/imx/imx-media-csi.c | 27 ---
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index c9110dd39a49..0d494a7db211 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1407,19 +1407,6 @@ static void csi_try_fmt(struct csi_priv *priv,
W_ALIGN, >format.height,
MIN_H, MAX_H, H_ALIGN, S_ALIGN);
- /* Reset crop and compose rectangles */
- crop->left = 0;
- crop->top = 0;
- crop->width = sdformat->format.width;
- crop->height = sdformat->format.height;
- if (sdformat->format.field == V4L2_FIELD_ALTERNATE)
- crop->height *= 2;
- csi_try_crop(priv, crop, cfg, >format, upstream_ep);
- compose->left = 0;
- compose->top = 0;
- compose->width = crop->width;
- compose->height = crop->height;
-
*cc = imx_media_find_mbus_format(sdformat->format.code,
CS_SEL_ANY, true);
if (!*cc) {
@@ -1435,6 +1422,20 @@ static void csi_try_fmt(struct csi_priv *priv,
imx_media_fill_default_mbus_fields(
>format, infmt,
priv->active_output_pad == CSI_SRC_PAD_DIRECT);
+
+ /* Reset crop and compose rectangles */
+ crop->left = 0;
+ crop->top = 0;
+ crop->width = sdformat->format.width;
+ crop->height = sdformat->format.height;
+ if (sdformat->format.field == V4L2_FIELD_ALTERNATE)
+ crop->height *= 2;
+ csi_try_crop(priv, crop, cfg, >format, upstream_ep);
+ compose->left = 0;
+ compose->top = 0;
+ compose->width = crop->width;
+ compose->height = crop->height;
+
break;
}
}
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 02/12] gpu: ipu-csi: Swap fields according to input/output field types
The function ipu_csi_init_interface() was inverting the F-bit for
NTSC case, in the CCIR_CODE_1/2 registers. The result being that
for NTSC bottom-top field order, the CSI would swap fields and
capture in top-bottom order.
Instead, base field swap on the field order of the input to the CSI,
and the field order of the requested output. If the input/output
fields are sequential but different, swap fields, otherwise do
not swap. This requires passing both the input and output mbus
frame formats to ipu_csi_init_interface().
Move this code to a new private function ipu_csi_set_bt_interlaced_codes()
that programs the CCIR_CODE_1/2 registers for interlaced BT.656 (and
possibly interlaced BT.1120 in the future).
When detecting input video standard from the input frame width/height,
make sure to double height if input field type is alternate, since
in that case input height only includes lines for one field.
Signed-off-by: Steve Longerbeam
---
Changes since v4:
- Cleaned up some convoluted code in ipu_csi_init_interface(), suggested
by Philipp Zabel.
- Fixed a regression in csi_setup(), caught by Philipp.
---
drivers/gpu/ipu-v3/ipu-csi.c | 119 +++---
drivers/staging/media/imx/imx-media-csi.c | 17 +---
include/video/imx-ipu-v3.h| 3 +-
3 files changed, 88 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
index aa0e30a2ba18..4a15e513fa05 100644
--- a/drivers/gpu/ipu-v3/ipu-csi.c
+++ b/drivers/gpu/ipu-v3/ipu-csi.c
@@ -325,6 +325,15 @@ static int mbus_code_to_bus_cfg(struct ipu_csi_bus_config
*cfg, u32 mbus_code,
return 0;
}
+/* translate alternate field mode based on given standard */
+static inline enum v4l2_field
+ipu_csi_translate_field(enum v4l2_field field, v4l2_std_id std)
+{
+ return (field != V4L2_FIELD_ALTERNATE) ? field :
+ ((std & V4L2_STD_525_60) ?
+V4L2_FIELD_SEQ_BT : V4L2_FIELD_SEQ_TB);
+}
+
/*
* Fill a CSI bus config struct from mbus_config and mbus_framefmt.
*/
@@ -374,22 +383,75 @@ static int fill_csi_bus_cfg(struct ipu_csi_bus_config
*csicfg,
return 0;
}
+static int ipu_csi_set_bt_interlaced_codes(struct ipu_csi *csi,
+ struct v4l2_mbus_framefmt *infmt,
+ struct v4l2_mbus_framefmt *outfmt,
+ v4l2_std_id std)
+{
+ enum v4l2_field infield, outfield;
+ bool swap_fields;
+
+ /* get translated field type of input and output */
+ infield = ipu_csi_translate_field(infmt->field, std);
+ outfield = ipu_csi_translate_field(outfmt->field, std);
+
+ /*
+* Write the H-V-F codes the CSI will match against the
+* incoming data for start/end of active and blanking
+* field intervals. If input and output field types are
+* sequential but not the same (one is SEQ_BT and the other
+* is SEQ_TB), swap the F-bit so that the CSI will capture
+* field 1 lines before field 0 lines.
+*/
+ swap_fields = (V4L2_FIELD_IS_SEQUENTIAL(infield) &&
+ V4L2_FIELD_IS_SEQUENTIAL(outfield) &&
+ infield != outfield);
+
+ if (!swap_fields) {
+ /*
+* Field0BlankEnd = 110, Field0BlankStart = 010
+* Field0ActiveEnd = 100, Field0ActiveStart = 000
+* Field1BlankEnd = 111, Field1BlankStart = 011
+* Field1ActiveEnd = 101, Field1ActiveStart = 001
+*/
+ ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
+ CSI_CCIR_CODE_1);
+ ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
+ } else {
+ dev_dbg(csi->ipu->dev, "capture field swap\n");
+
+ /* same as above but with F-bit inverted */
+ ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
+ CSI_CCIR_CODE_1);
+ ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
+ }
+
+ ipu_csi_write(csi, 0xFF, CSI_CCIR_CODE_3);
+
+ return 0;
+}
+
+
int ipu_csi_init_interface(struct ipu_csi *csi,
struct v4l2_mbus_config *mbus_cfg,
- struct v4l2_mbus_framefmt *mbus_fmt)
+ struct v4l2_mbus_framefmt *infmt,
+ struct v4l2_mbus_framefmt *outfmt)
{
struct ipu_csi_bus_config cfg;
unsigned long flags;
u32 width, height, data = 0;
+ v4l2_std_id std;
int ret;
- ret = fill_csi_bus_cfg(, mbus_cfg, mbus_fmt);
+ ret = fill_csi_bus_cfg(, mbus_cfg, infmt);
if (ret < 0)
return ret;
/* set default sensor frame width and height */
- width = mbus_fmt->width;
- height = mbus_fmt->height;
+ width = infmt->width;
+ height = infmt->height;
+
[PATCH v5 04/12] media: imx: Fix field negotiation
IDMAC interlaced scan, a.k.a. interweave, should be enabled in the
IDMAC output channels only if the IDMAC output pad field type is
'seq-bt' or 'seq-tb', and field type at the capture interface is
'interlaced*'.
V4L2_FIELD_HAS_BOTH() macro should not be used on the input to determine
enabling interlaced/interweave scan. That macro includes the 'interlaced'
field types, and in those cases the data is already interweaved with
top/bottom field lines.
The CSI will capture whole frames when the source specifies alternate
field mode. So the CSI also enables interweave for alternate input
field type and the field type at capture interface is interlaced.
Fix the logic for setting field type in try_fmt in CSI entity.
The behavior should be:
- No restrictions on field type at sink pad.
- At the output pads, allow sequential fields in TB order, if the sink pad
field type is sequential or alternate. Otherwise passthrough the field
type from sink to source pad.
Move this logic to new function csi_try_field().
These changes result in the following allowed field transformations
from CSI sink -> source pads (all other field types at sink are passed
through to source):
seq-tb -> seq-tb
seq-bt -> seq-tb
alternate -> seq-tb
In a future patch, the CSI sink -> source will allow:
seq-tb -> seq-bt
seq-bt -> seq-bt
alternate -> seq-bt
This will require supporting interweave with top/bottom line swapping.
Until then seq-bt is not allowed at the CSI source pad because there is
no way to swap top/bottom lines when interweaving to INTERLACED_BT --
note that despite the name, INTERLACED_BT is top-bottom order in memory.
The BT in this case refers to field dominance: the bottom lines are
older in time than the top lines.
The capture interface device allows selecting IDMAC interweave by
choosing INTERLACED_TB if the CSI/PRPENCVF source pad is seq-tb and
INTERLACED_BT if the source pad is seq-bt (for future support of seq-bt).
Signed-off-by: Steve Longerbeam
Reviewed-by: Philipp Zabel
---
drivers/staging/media/imx/imx-ic-prpencvf.c | 21 --
drivers/staging/media/imx/imx-media-capture.c | 14
drivers/staging/media/imx/imx-media-csi.c | 64 ++-
3 files changed, 76 insertions(+), 23 deletions(-)
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c
b/drivers/staging/media/imx/imx-ic-prpencvf.c
index af7224846bd5..1a03d4c9d7b8 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -354,12 +354,13 @@ static int prp_setup_channel(struct prp_priv *priv,
{
struct imx_media_video_dev *vdev = priv->vdev;
const struct imx_media_pixfmt *outcc;
- struct v4l2_mbus_framefmt *infmt;
+ struct v4l2_mbus_framefmt *outfmt;
unsigned int burst_size;
struct ipu_image image;
+ bool interweave;
int ret;
- infmt = >format_mbus[PRPENCVF_SINK_PAD];
+ outfmt = >format_mbus[PRPENCVF_SRC_PAD];
outcc = vdev->cc;
ipu_cpmem_zero(channel);
@@ -369,6 +370,15 @@ static int prp_setup_channel(struct prp_priv *priv,
image.rect.width = image.pix.width;
image.rect.height = image.pix.height;
+ /*
+* If the field type at capture interface is interlaced, and
+* the output IDMAC pad is sequential, enable interweave at
+* the IDMAC output channel.
+*/
+ interweave = V4L2_FIELD_IS_INTERLACED(image.pix.field) &&
+ V4L2_FIELD_IS_SEQUENTIAL(outfmt->field) &&
+ channel == priv->out_ch;
+
if (rot_swap_width_height) {
swap(image.pix.width, image.pix.height);
swap(image.rect.width, image.rect.height);
@@ -409,9 +419,7 @@ static int prp_setup_channel(struct prp_priv *priv,
if (rot_mode)
ipu_cpmem_set_rotation(channel, rot_mode);
- if (image.pix.field == V4L2_FIELD_NONE &&
- V4L2_FIELD_HAS_BOTH(infmt->field) &&
- channel == priv->out_ch)
+ if (interweave)
ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline,
image.pix.pixelformat);
@@ -839,8 +847,7 @@ static void prp_try_fmt(struct prp_priv *priv,
infmt = __prp_get_fmt(priv, cfg, PRPENCVF_SINK_PAD, sdformat->which);
if (sdformat->pad == PRPENCVF_SRC_PAD) {
- if (sdformat->format.field != V4L2_FIELD_NONE)
- sdformat->format.field = infmt->field;
+ sdformat->format.field = infmt->field;
prp_bound_align_output(>format, infmt,
priv->rot_mode);
diff --git a/drivers/staging/media/imx/imx-media-capture.c
b/drivers/staging/media/imx/imx-media-capture.c
index b37e1186eb2f..01ec9443de55 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -239,6 +239,20 @@ static int capture_try_fmt_vid_cap(struct file *file, void
*fh,
[PATCH v5 06/12] media: imx-csi: Double crop height for alternate fields at sink
If the incoming sink field type is alternate, the reset crop height
and crop height bounds must be set to twice the incoming height,
because in alternate field mode, upstream will report only the
lines for a single field, and the CSI captures the whole frame.
Signed-off-by: Steve Longerbeam
Reviewed-by: Philipp Zabel
---
drivers/staging/media/imx/imx-media-csi.c | 20 +++-
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index 8f52428d2c75..d5b0f8a66750 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1140,6 +1140,8 @@ static void csi_try_crop(struct csi_priv *priv,
struct v4l2_mbus_framefmt *infmt,
struct v4l2_fwnode_endpoint *upstream_ep)
{
+ u32 in_height;
+
crop->width = min_t(__u32, infmt->width, crop->width);
if (crop->left + crop->width > infmt->width)
crop->left = infmt->width - crop->width;
@@ -1147,6 +1149,10 @@ static void csi_try_crop(struct csi_priv *priv,
crop->left &= ~0x3;
crop->width &= ~0x7;
+ in_height = infmt->height;
+ if (infmt->field == V4L2_FIELD_ALTERNATE)
+ in_height *= 2;
+
/*
* FIXME: not sure why yet, but on interlaced bt.656,
* changing the vertical cropping causes loss of vertical
@@ -1156,12 +1162,12 @@ static void csi_try_crop(struct csi_priv *priv,
if (upstream_ep->bus_type == V4L2_MBUS_BT656 &&
(V4L2_FIELD_HAS_BOTH(infmt->field) ||
infmt->field == V4L2_FIELD_ALTERNATE)) {
- crop->height = infmt->height;
- crop->top = (infmt->height == 480) ? 2 : 0;
+ crop->height = in_height;
+ crop->top = (in_height == 480) ? 2 : 0;
} else {
- crop->height = min_t(__u32, infmt->height, crop->height);
- if (crop->top + crop->height > infmt->height)
- crop->top = infmt->height - crop->height;
+ crop->height = min_t(__u32, in_height, crop->height);
+ if (crop->top + crop->height > in_height)
+ crop->top = in_height - crop->height;
}
}
@@ -1401,6 +1407,8 @@ static void csi_try_fmt(struct csi_priv *priv,
crop->top = 0;
crop->width = sdformat->format.width;
crop->height = sdformat->format.height;
+ if (sdformat->format.field == V4L2_FIELD_ALTERNATE)
+ crop->height *= 2;
csi_try_crop(priv, crop, cfg, >format, upstream_ep);
compose->left = 0;
compose->top = 0;
@@ -1528,6 +1536,8 @@ static int csi_get_selection(struct v4l2_subdev *sd,
sel->r.top = 0;
sel->r.width = infmt->width;
sel->r.height = infmt->height;
+ if (infmt->field == V4L2_FIELD_ALTERNATE)
+ sel->r.height *= 2;
break;
case V4L2_SEL_TGT_CROP:
sel->r = *crop;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 11/12] media: imx: Allow interweave with top/bottom lines swapped
Allow sequential->interlaced interweaving but with top/bottom
lines swapped to the output buffer.
This can be accomplished by adding one line length to IDMAC output
channel address, with a negative line length for the interlace offset.
This is to allow the seq-bt -> interlaced-bt transformation, where
bottom lines are still dominant (older in time) but with top lines
first in the interweaved output buffer.
With this support, the CSI can now allow seq-bt at its source pads,
e.g. the following transformations are allowed in CSI from sink to
source:
seq-tb -> seq-bt
seq-bt -> seq-bt
alternate -> seq-bt
Suggested-by: Philipp Zabel
Signed-off-by: Steve Longerbeam
Reviewed-by: Philipp Zabel
---
Changes since v4:
- Removed interweave_offset and replace with boolean interweave_swap,
suggested by Philipp Zabel.
---
drivers/staging/media/imx/imx-ic-prpencvf.c | 25 +
drivers/staging/media/imx/imx-media-csi.c | 40 ++---
2 files changed, 54 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c
b/drivers/staging/media/imx/imx-ic-prpencvf.c
index cf76b0432371..33ada6612fee 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -106,6 +106,7 @@ struct prp_priv {
u32 frame_sequence; /* frame sequence counter */
bool last_eof; /* waiting for last EOF at stream off */
bool nfb4eof;/* NFB4EOF encountered during streaming */
+ bool interweave_swap; /* swap top/bottom lines when interweaving */
struct completion last_eof_comp;
};
@@ -235,6 +236,9 @@ static void prp_vb2_buf_done(struct prp_priv *priv, struct
ipuv3_channel *ch)
if (ipu_idmac_buffer_is_ready(ch, priv->ipu_buf_num))
ipu_idmac_clear_buffer(ch, priv->ipu_buf_num);
+ if (priv->interweave_swap && ch == priv->out_ch)
+ phys += vdev->fmt.fmt.pix.bytesperline;
+
ipu_cpmem_set_buffer(ch, priv->ipu_buf_num, phys);
}
@@ -376,8 +380,9 @@ static int prp_setup_channel(struct prp_priv *priv,
* the IDMAC output channel.
*/
interweave = V4L2_FIELD_IS_INTERLACED(image.pix.field) &&
- V4L2_FIELD_IS_SEQUENTIAL(outfmt->field) &&
- channel == priv->out_ch;
+ V4L2_FIELD_IS_SEQUENTIAL(outfmt->field);
+ priv->interweave_swap = interweave &&
+ image.pix.field == V4L2_FIELD_INTERLACED_BT;
if (rot_swap_width_height) {
swap(image.pix.width, image.pix.height);
@@ -388,6 +393,11 @@ static int prp_setup_channel(struct prp_priv *priv,
(image.pix.width * outcc->bpp) >> 3;
}
+ if (priv->interweave_swap && channel == priv->out_ch) {
+ /* start interweave scan at 1st top line (2nd line) */
+ image.rect.top = 1;
+ }
+
image.phys0 = addr0;
image.phys1 = addr1;
@@ -396,8 +406,8 @@ static int prp_setup_channel(struct prp_priv *priv,
* channels for planar 4:2:0 (but not when enabling IDMAC
* interweaving, they are incompatible).
*/
- if (!interweave && (channel == priv->out_ch ||
- channel == priv->rot_out_ch)) {
+ if ((channel == priv->out_ch && !interweave) ||
+ channel == priv->rot_out_ch) {
switch (image.pix.pixelformat) {
case V4L2_PIX_FMT_YUV420:
case V4L2_PIX_FMT_YVU420:
@@ -424,8 +434,11 @@ static int prp_setup_channel(struct prp_priv *priv,
if (rot_mode)
ipu_cpmem_set_rotation(channel, rot_mode);
- if (interweave)
- ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline,
+ if (interweave && channel == priv->out_ch)
+ ipu_cpmem_interlaced_scan(channel,
+ priv->interweave_swap ?
+ -image.pix.bytesperline :
+ image.pix.bytesperline,
image.pix.pixelformat);
ret = ipu_ic_task_idma_init(priv->ic, channel,
diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index 0d494a7db211..73c9f3ae4221 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -114,6 +114,7 @@ struct csi_priv {
u32 frame_sequence; /* frame sequence counter */
bool last_eof; /* waiting for last EOF at stream off */
bool nfb4eof;/* NFB4EOF encountered during streaming */
+ bool interweave_swap; /* swap top/bottom lines when interweaving */
struct completion last_eof_comp;
};
@@ -286,6 +287,9 @@ static void csi_vb2_buf_done(struct csi_priv *priv)
if (ipu_idmac_buffer_is_ready(priv->idmac_ch, priv->ipu_buf_num))
ipu_idmac_clear_buffer(priv->idmac_ch, priv->ipu_buf_num);
+
[PATCH v5 08/12] media: imx-csi: Allow skipping odd chroma rows for YVU420
Skip writing U/V components to odd rows for YVU420 in addition to YUV420 and NV12. Signed-off-by: Steve Longerbeam Reviewed-by: Philipp Zabel --- drivers/staging/media/imx/imx-media-csi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c index 7e648fc9626a..c9110dd39a49 100644 --- a/drivers/staging/media/imx/imx-media-csi.c +++ b/drivers/staging/media/imx/imx-media-csi.c @@ -452,6 +452,7 @@ static int csi_idmac_setup_channel(struct csi_priv *priv) passthrough_bits = 16; break; case V4L2_PIX_FMT_YUV420: + case V4L2_PIX_FMT_YVU420: case V4L2_PIX_FMT_NV12: burst_size = (image.pix.width & 0x3f) ? ((image.pix.width & 0x1f) ? -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 09/12] media: imx: vdic: rely on VDIC for correct field order
prepare_vdi_in_buffers() was setting up the dma pointers as if the
VDIC is always programmed to receive the fields in bottom-top order,
i.e. as if ipu_vdi_set_field_order() only programs BT order in the VDIC.
But that's not true, ipu_vdi_set_field_order() is working correctly.
So fix prepare_vdi_in_buffers() to give the VDIC the fields in whatever
order they were received by the video source, and rely on the VDIC to
sort out which is top and which is bottom.
Signed-off-by: Steve Longerbeam
---
drivers/staging/media/imx/imx-media-vdic.c | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-vdic.c
b/drivers/staging/media/imx/imx-media-vdic.c
index 482250d47e7c..4a890714193e 100644
--- a/drivers/staging/media/imx/imx-media-vdic.c
+++ b/drivers/staging/media/imx/imx-media-vdic.c
@@ -219,26 +219,18 @@ static void __maybe_unused prepare_vdi_in_buffers(struct
vdic_priv *priv,
switch (priv->fieldtype) {
case V4L2_FIELD_SEQ_TB:
- prev_phys = vb2_dma_contig_plane_dma_addr(prev_vb, 0);
- curr_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0) + fs;
- next_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0);
- break;
case V4L2_FIELD_SEQ_BT:
prev_phys = vb2_dma_contig_plane_dma_addr(prev_vb, 0) + fs;
curr_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0);
next_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0) + fs;
break;
+ case V4L2_FIELD_INTERLACED_TB:
case V4L2_FIELD_INTERLACED_BT:
+ case V4L2_FIELD_INTERLACED:
prev_phys = vb2_dma_contig_plane_dma_addr(prev_vb, 0) + is;
curr_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0);
next_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0) + is;
break;
- default:
- /* assume V4L2_FIELD_INTERLACED_TB */
- prev_phys = vb2_dma_contig_plane_dma_addr(prev_vb, 0);
- curr_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0) + is;
- next_phys = vb2_dma_contig_plane_dma_addr(curr_vb, 0);
- break;
}
ipu_cpmem_set_buffer(priv->vdi_in_ch_p, 0, prev_phys);
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 05/12] media: imx-csi: Input connections to CSI should be optional
Some imx platforms do not have fwnode connections to all CSI input
ports, and should not be treated as an error. This includes the
imx6q SabreAuto, which has no connections to ipu1_csi1 and ipu2_csi0.
Return -ENOTCONN in imx_csi_parse_endpoint() so that v4l2-fwnode
endpoint parsing will not treat an unconnected endpoint as an error.
Fixes: c893500a16baf ("media: imx: csi: Register a subdev notifier")
Signed-off-by: Steve Longerbeam
---
drivers/staging/media/imx/imx-media-csi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index 176978c7dfe7..8f52428d2c75 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1813,7 +1813,7 @@ static int imx_csi_parse_endpoint(struct device *dev,
struct v4l2_fwnode_endpoint *vep,
struct v4l2_async_subdev *asd)
{
- return fwnode_device_is_available(asd->match.fwnode) ? 0 : -EINVAL;
+ return fwnode_device_is_available(asd->match.fwnode) ? 0 : -ENOTCONN;
}
static int imx_csi_async_register(struct csi_priv *priv)
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 07/12] media: imx: interweave and odd-chroma-row skip are incompatible
If IDMAC interweaving is enabled in a write channel, the channel must
write the odd chroma rows for 4:2:0 formats. Skipping writing the odd
chroma rows produces corrupted captured 4:2:0 images when interweave
is enabled.
Reported-by: Krzysztof Hałasa
Signed-off-by: Steve Longerbeam
Reviewed-by: Philipp Zabel
---
drivers/staging/media/imx/imx-ic-prpencvf.c | 9 +++--
drivers/staging/media/imx/imx-media-csi.c | 8 ++--
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c
b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 1a03d4c9d7b8..cf76b0432371 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -391,12 +391,17 @@ static int prp_setup_channel(struct prp_priv *priv,
image.phys0 = addr0;
image.phys1 = addr1;
- if (channel == priv->out_ch || channel == priv->rot_out_ch) {
+ /*
+* Skip writing U and V components to odd rows in the output
+* channels for planar 4:2:0 (but not when enabling IDMAC
+* interweaving, they are incompatible).
+*/
+ if (!interweave && (channel == priv->out_ch ||
+ channel == priv->rot_out_ch)) {
switch (image.pix.pixelformat) {
case V4L2_PIX_FMT_YUV420:
case V4L2_PIX_FMT_YVU420:
case V4L2_PIX_FMT_NV12:
- /* Skip writing U and V components to odd rows */
ipu_cpmem_skip_odd_chroma_rows(channel);
break;
}
diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index d5b0f8a66750..7e648fc9626a 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -457,8 +457,12 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
((image.pix.width & 0x1f) ?
((image.pix.width & 0xf) ? 8 : 16) : 32) : 64;
passthrough_bits = 16;
- /* Skip writing U and V components to odd rows */
- ipu_cpmem_skip_odd_chroma_rows(priv->idmac_ch);
+ /*
+* Skip writing U and V components to odd rows (but not
+* when enabling IDMAC interweaving, they are incompatible).
+*/
+ if (!interweave)
+ ipu_cpmem_skip_odd_chroma_rows(priv->idmac_ch);
break;
case V4L2_PIX_FMT_YUYV:
case V4L2_PIX_FMT_UYVY:
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 03/12] gpu: ipu-v3: Add planar support to interlaced scan
To support interlaced scan with planar formats, cpmem SLUV must
be programmed with the correct chroma line stride. For full and
partial planar 4:2:2 (YUV422P, NV16), chroma line stride must
be doubled. For full and partial planar 4:2:0 (YUV420, YVU420, NV12),
chroma line stride must _not_ be doubled, since a single chroma line
is shared by two luma lines.
Signed-off-by: Steve Longerbeam
Reviewed-by: Philipp Zabel
Acked-by: Philipp Zabel
---
drivers/gpu/ipu-v3/ipu-cpmem.c | 26 +++--
drivers/staging/media/imx/imx-ic-prpencvf.c | 3 ++-
drivers/staging/media/imx/imx-media-csi.c | 3 ++-
include/video/imx-ipu-v3.h | 3 ++-
4 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c
index a9d2501500a1..d41df8034c5b 100644
--- a/drivers/gpu/ipu-v3/ipu-cpmem.c
+++ b/drivers/gpu/ipu-v3/ipu-cpmem.c
@@ -273,9 +273,10 @@ void ipu_cpmem_set_uv_offset(struct ipuv3_channel *ch, u32
u_off, u32 v_off)
}
EXPORT_SYMBOL_GPL(ipu_cpmem_set_uv_offset);
-void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride)
+void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride,
+ u32 pixelformat)
{
- u32 ilo, sly;
+ u32 ilo, sly, sluv;
if (stride < 0) {
stride = -stride;
@@ -286,9 +287,30 @@ void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch,
int stride)
sly = (stride * 2) - 1;
+ switch (pixelformat) {
+ case V4L2_PIX_FMT_YUV420:
+ case V4L2_PIX_FMT_YVU420:
+ sluv = stride / 2 - 1;
+ break;
+ case V4L2_PIX_FMT_NV12:
+ sluv = stride - 1;
+ break;
+ case V4L2_PIX_FMT_YUV422P:
+ sluv = stride - 1;
+ break;
+ case V4L2_PIX_FMT_NV16:
+ sluv = stride * 2 - 1;
+ break;
+ default:
+ sluv = 0;
+ break;
+ }
+
ipu_ch_param_write_field(ch, IPU_FIELD_SO, 1);
ipu_ch_param_write_field(ch, IPU_FIELD_ILO, ilo);
ipu_ch_param_write_field(ch, IPU_FIELD_SLY, sly);
+ if (sluv)
+ ipu_ch_param_write_field(ch, IPU_FIELD_SLUV, sluv);
};
EXPORT_SYMBOL_GPL(ipu_cpmem_interlaced_scan);
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c
b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 28f41caba05d..af7224846bd5 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -412,7 +412,8 @@ static int prp_setup_channel(struct prp_priv *priv,
if (image.pix.field == V4L2_FIELD_NONE &&
V4L2_FIELD_HAS_BOTH(infmt->field) &&
channel == priv->out_ch)
- ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline);
+ ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline,
+ image.pix.pixelformat);
ret = ipu_ic_task_idma_init(priv->ic, channel,
image.pix.width, image.pix.height,
diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index 7ecbd4d76d09..4aa20ae72608 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -512,7 +512,8 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
if (image.pix.field == V4L2_FIELD_NONE &&
V4L2_FIELD_HAS_BOTH(infmt->field))
ipu_cpmem_interlaced_scan(priv->idmac_ch,
- image.pix.bytesperline);
+ image.pix.bytesperline,
+ image.pix.pixelformat);
ipu_idmac_set_double_buffer(priv->idmac_ch, true);
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index f44a35192313..e888c66b9d9d 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -255,7 +255,8 @@ void ipu_cpmem_set_stride(struct ipuv3_channel *ch, int
stride);
void ipu_cpmem_set_high_priority(struct ipuv3_channel *ch);
void ipu_cpmem_set_buffer(struct ipuv3_channel *ch, int bufnum, dma_addr_t
buf);
void ipu_cpmem_set_uv_offset(struct ipuv3_channel *ch, u32 u_off, u32 v_off);
-void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride);
+void ipu_cpmem_interlaced_scan(struct ipuv3_channel *ch, int stride,
+ u32 pixelformat);
void ipu_cpmem_set_axi_id(struct ipuv3_channel *ch, u32 id);
int ipu_cpmem_get_burstsize(struct ipuv3_channel *ch);
void ipu_cpmem_set_burstsize(struct ipuv3_channel *ch, int burstsize);
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: ad7280a: Lines should not end with a '(' - style
>(There is a linux-...@googlegroups.com mailing list >that bounces when I reply, so I removed it from the >cc list) Sorry. I think this may be because my HTML mode in gmail was enabled. > I wrote that to encourage you to do more than > what checkpatch says. > I just moved code around and reduced duplication. > There are many similar opportunities for code > refactoring in staging. Thank you :-) I will put effort to improve these points. > You could test what I wrote, add a good commit > message with a subject like: > > [PATCH V2] staging: iio: ad7280a: Refactor > > with a commit message that describes the changes and > perhaps shows the object size difference using > > $ size I will do that. :-) > Are you doing this for a class assignment? Yes, I am. I am into a group that aims to contribute to opensource projects and we chose the Linux kernel. Our mentor suggested us to contribute to the linux-iio Thank you On Tue, Oct 16, 2018 at 8:08 PM Joe Perches wrote: > > (There is a linux-...@googlegroups.com mailing list > that bounces when I reply, so I removed it from the > cc list) > > On Tue, 2018-10-16 at 19:48 -0300, Giuliano Belinassi wrote: > > Hello, > > Thank you for your review :-). > > Sorry, but I am a newbie on this, and now I am confused about my next > > step. Should I make a v2 based on your changes, or do you want to submit > > your changes? > > I wrote that to encourage you to do more than > what checkpatch says. > > I just moved code around and reduced duplication. > > There are many similar opportunities for code > refactoring in staging. > > You could test what I wrote, add a good commit > message with a subject like: > > [PATCH V2] staging: iio: ad7280a: Refactor > > with a commit message that describes the changes and > perhaps shows the object size difference using > > $ size > > Maybe add a Suggested-by: tag if it pleases you, but > what I did is trivial and I think it's unnecessary. > > Are you doing this for a class assignment? > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: ad7280a: Lines should not end with a '(' - style
(There is a linux-...@googlegroups.com mailing list that bounces when I reply, so I removed it from the cc list) On Tue, 2018-10-16 at 19:48 -0300, Giuliano Belinassi wrote: > Hello, > Thank you for your review :-). > Sorry, but I am a newbie on this, and now I am confused about my next > step. Should I make a v2 based on your changes, or do you want to submit > your changes? I wrote that to encourage you to do more than what checkpatch says. I just moved code around and reduced duplication. There are many similar opportunities for code refactoring in staging. You could test what I wrote, add a good commit message with a subject like: [PATCH V2] staging: iio: ad7280a: Refactor with a commit message that describes the changes and perhaps shows the object size difference using $ size Maybe add a Suggested-by: tag if it pleases you, but what I did is trivial and I think it's unnecessary. Are you doing this for a class assignment? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: ad7280a: Lines should not end with a '(' - style
On Tue, 2018-10-16 at 18:09 -0300, Giuliano Belinassi wrote:
> A number of macro calls cause a checkpatch issue:
>
> "Lines should not end with a '('"
>
> This was fixed by moving the first '(' token to the line of the first
> argument.
Please try to make the code more readable instead of
following mindless checkpatch messages.
For instance, this could be shorter, simpler, smaller
object code size, and easier to humans to read as:
---
drivers/staging/iio/adc/ad7280a.c | 68 +--
1 file changed, 30 insertions(+), 38 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7280a.c
b/drivers/staging/iio/adc/ad7280a.c
index 58420dcb406d..a69ae76b5616 100644
--- a/drivers/staging/iio/adc/ad7280a.c
+++ b/drivers/staging/iio/adc/ad7280a.c
@@ -701,46 +701,38 @@ static irqreturn_t ad7280_event_handler(int irq, void
*private)
goto out;
for (i = 0; i < st->scan_cnt; i++) {
- if (((channels[i] >> 23) & 0xF) <= AD7280A_CELL_VOLTAGE_6) {
- if (((channels[i] >> 11) & 0xFFF) >=
- st->cell_threshhigh)
- iio_push_event(indio_dev,
- IIO_EVENT_CODE(IIO_VOLTAGE,
- 1,
- 0,
- IIO_EV_DIR_RISING,
- IIO_EV_TYPE_THRESH,
- 0, 0, 0),
- iio_get_time_ns(indio_dev));
- else if (((channels[i] >> 11) & 0xFFF) <=
- st->cell_threshlow)
- iio_push_event(indio_dev,
- IIO_EVENT_CODE(IIO_VOLTAGE,
- 1,
- 0,
- IIO_EV_DIR_FALLING,
- IIO_EV_TYPE_THRESH,
- 0, 0, 0),
- iio_get_time_ns(indio_dev));
+ unsigned int voltage = (channels[i] >> 23) & 0xF;
+ unsigned int val = (channels[i] >> 11) & 0xFFF;
+ u64 code = 0;
+
+ if (voltage <= AD7280A_CELL_VOLTAGE_6) {
+ if (val >= st->cell_threshhigh) {
+ code = IIO_EVENT_CODE(IIO_VOLTAGE, 1, 0,
+ IIO_EV_DIR_RISING,
+ IIO_EV_TYPE_THRESH,
+ 0, 0, 0);
+ } else if (val <= st->cell_threshlow) {
+ code = IIO_EVENT_CODE(IIO_VOLTAGE, 1, 0,
+ IIO_EV_DIR_FALLING,
+ IIO_EV_TYPE_THRESH,
+ 0, 0, 0);
+ } else {
+ continue;
+ }
} else {
- if (((channels[i] >> 11) & 0xFFF) >= st->aux_threshhigh)
- iio_push_event(indio_dev,
- IIO_UNMOD_EVENT_CODE(
- IIO_TEMP,
- 0,
- IIO_EV_TYPE_THRESH,
- IIO_EV_DIR_RISING),
- iio_get_time_ns(indio_dev));
- else if (((channels[i] >> 11) & 0xFFF) <=
- st->aux_threshlow)
- iio_push_event(indio_dev,
- IIO_UNMOD_EVENT_CODE(
- IIO_TEMP,
- 0,
- IIO_EV_TYPE_THRESH,
- IIO_EV_DIR_FALLING),
- iio_get_time_ns(indio_dev));
+ if (val >= st->aux_threshhigh) {
+ code = IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
+ IIO_EV_TYPE_THRESH,
+ IIO_EV_DIR_RISING);
+ } else if (val <= st->aux_threshlow) {
+ code = IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
+
[PATCH] staging: iio: ad7280a: Lines should not end with a '(' - style
A number of macro calls cause a checkpatch issue:
"Lines should not end with a '('"
This was fixed by moving the first '(' token to the line of the first
argument.
Signed-off-by: Giuliano Belinassi
---
drivers/staging/iio/adc/ad7280a.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7280a.c
b/drivers/staging/iio/adc/ad7280a.c
index 58420dcb406d..f7df987412d7 100644
--- a/drivers/staging/iio/adc/ad7280a.c
+++ b/drivers/staging/iio/adc/ad7280a.c
@@ -725,8 +725,8 @@ static irqreturn_t ad7280_event_handler(int irq, void
*private)
} else {
if (((channels[i] >> 11) & 0xFFF) >= st->aux_threshhigh)
iio_push_event(indio_dev,
- IIO_UNMOD_EVENT_CODE(
- IIO_TEMP,
+ IIO_UNMOD_EVENT_CODE
+ (IIO_TEMP,
0,
IIO_EV_TYPE_THRESH,
IIO_EV_DIR_RISING),
@@ -734,8 +734,8 @@ static irqreturn_t ad7280_event_handler(int irq, void
*private)
else if (((channels[i] >> 11) & 0xFFF) <=
st->aux_threshlow)
iio_push_event(indio_dev,
- IIO_UNMOD_EVENT_CODE(
- IIO_TEMP,
+ IIO_UNMOD_EVENT_CODE
+ (IIO_TEMP,
0,
IIO_EV_TYPE_THRESH,
IIO_EV_DIR_FALLING),
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: adc: Add comments to prevent checks corrections
On Tue, 2018-10-16 at 20:06 +0200, Slawomir Stepien wrote:
> On paź 15, 2018 18:25, Gabriel Capella wrote:
> > This patch adds 3 comments in 2 different files.
> > These comments warn to don't correct the checks of type:
> > "CHECK: spaces preferred around that '-'"
> >
> > Signed-off-by: Gabriel Capella
> > ---
> > drivers/staging/iio/adc/ad7192.c | 1 +
> > drivers/staging/iio/adc/ad7280a.c | 2 ++
> > 2 files changed, 3 insertions(+)
>
> I have this simpler solution...let's focus our efforts on moving the two
> drivers
> out of staging. This will prevent the CHK to appear:
>
> Cut from checkpatch.pl:
>
> if ($realfile =~ m@^(?:drivers/net/|net/|drivers/staging/)@) {
> $check = 1;
>
> Existing driver out of staging, with this "problem":
> $ ./scripts/checkpatch.pl --types SPACING --file drivers/iio/adc/ad7793.c
>
> total: 0 errors, 0 warnings, 827 lines checked
>
> drivers/iio/adc/ad7793.c has no obvious style problems and is ready for
> submission.
>
> NOTE: Used message types: SPACING
>
> In my opinion it would stop this incorrect patches. -
>
> I have also this changes to checkpatch.pl:
> https://github.com/s-stepien/linux-1/commit/c976a31b392393fb417f2d9e2ec9639bc226ad0b
> and usage:
> https://github.com/s-stepien/linux-1/commit/1adc0428b496f44f6a931637084bb619ddd9992d
> but I'm not that sure it is the best way to go.
>
> What do you all think?
I think the suggested form is somewhat cryptic
/* checkpatch- */
and the new checkpatch code is somewhat balky.
It also allows only a single type to ignore per line.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: adc: Add comments to prevent checks corrections
On paź 15, 2018 18:25, Gabriel Capella wrote:
> This patch adds 3 comments in 2 different files.
> These comments warn to don't correct the checks of type:
> "CHECK: spaces preferred around that '-'"
>
> Signed-off-by: Gabriel Capella
> ---
> drivers/staging/iio/adc/ad7192.c | 1 +
> drivers/staging/iio/adc/ad7280a.c | 2 ++
> 2 files changed, 3 insertions(+)
I have this simpler solution...let's focus our efforts on moving the two drivers
out of staging. This will prevent the CHK to appear:
Cut from checkpatch.pl:
if ($realfile =~ m@^(?:drivers/net/|net/|drivers/staging/)@) {
$check = 1;
Existing driver out of staging, with this "problem":
$ ./scripts/checkpatch.pl --types SPACING --file drivers/iio/adc/ad7793.c
total: 0 errors, 0 warnings, 827 lines checked
drivers/iio/adc/ad7793.c has no obvious style problems and is ready for
submission.
NOTE: Used message types: SPACING
In my opinion it would stop this incorrect patches.
I have also this changes to checkpatch.pl:
https://github.com/s-stepien/linux-1/commit/c976a31b392393fb417f2d9e2ec9639bc226ad0b
and usage:
https://github.com/s-stepien/linux-1/commit/1adc0428b496f44f6a931637084bb619ddd9992d
but I'm not that sure it is the best way to go.
What do you all think?
--
Slawomir Stepien
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging iio/adc: fixes parenthesis alignment
Fixes close parenthesis alignment to match open parenthesis at iio/drivers/staging/iio/adc/ad7606.c line 379. Signed-of-by: Marcelo Schmitt --- drivers/staging/iio/adc/ad7606.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c index 0b728b6ea135..230faae38c12 100644 --- a/drivers/staging/iio/adc/ad7606.c +++ b/drivers/staging/iio/adc/ad7606.c @@ -376,7 +376,7 @@ static int ad7606_request_gpios(struct ad7606_state *st) return 0; st->gpio_os = devm_gpiod_get_array_optional(dev, "oversampling-ratio", - GPIOD_OUT_LOW); + GPIOD_OUT_LOW); return PTR_ERR_OR_ZERO(st->gpio_os); } -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: ad7816: Switch to the gpio descriptor interface
On 10/16/2018 04:46 PM, Nishad Kamdar wrote:
> Use the gpiod interface for rdwr_pin, convert_pin and busy_pin
> instead of the deprecated old non-descriptor interface.
>
> Signed-off-by: Nishad Kamdar
Hi,
Thanks for the patch, this looks good.
One thing about the error messages though.
> + chip->rdwr_pin = devm_gpiod_get(_dev->dev, "rdwr", GPIOD_IN);
> + if (IS_ERR(chip->rdwr_pin)) {
> + ret = PTR_ERR(chip->rdwr_pin);
> dev_err(_dev->dev, "Fail to request rdwr gpio PIN %d.\n",
> - chip->rdwr_pin);
> + ret);
This previously showed the pin number which has now been replaced with the
error code. The message doesn't make that much sense semantically anymore.
Maybe replace it with something like
"Failed to request rdwr GPIO: %d\n", ret
> return ret;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: iio: ad7816: Switch to the gpio descriptor interface
Use the gpiod interface for rdwr_pin, convert_pin and busy_pin
instead of the deprecated old non-descriptor interface.
Signed-off-by: Nishad Kamdar
---
drivers/staging/iio/adc/ad7816.c | 72 +---
1 file changed, 30 insertions(+), 42 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
index bf76a8620bdb..190b751af866 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -7,7 +7,7 @@
*/
#include
-#include
+#include
#include
#include
#include
@@ -44,9 +44,9 @@
struct ad7816_chip_info {
struct spi_device *spi_dev;
- u16 rdwr_pin;
- u16 convert_pin;
- u16 busy_pin;
+ struct gpio_desc *rdwr_pin;
+ struct gpio_desc *convert_pin;
+ struct gpio_desc *busy_pin;
u8 oti_data[AD7816_CS_MAX + 1];
u8 channel_id; /* 0 always be temperature */
u8 mode;
@@ -61,28 +61,28 @@ static int ad7816_spi_read(struct ad7816_chip_info *chip,
u16 *data)
int ret = 0;
__be16 buf;
- gpio_set_value(chip->rdwr_pin, 1);
- gpio_set_value(chip->rdwr_pin, 0);
+ gpiod_set_value(chip->rdwr_pin, 1);
+ gpiod_set_value(chip->rdwr_pin, 0);
ret = spi_write(spi_dev, >channel_id, sizeof(chip->channel_id));
if (ret < 0) {
dev_err(_dev->dev, "SPI channel setting error\n");
return ret;
}
- gpio_set_value(chip->rdwr_pin, 1);
+ gpiod_set_value(chip->rdwr_pin, 1);
if (chip->mode == AD7816_PD) { /* operating mode 2 */
- gpio_set_value(chip->convert_pin, 1);
- gpio_set_value(chip->convert_pin, 0);
+ gpiod_set_value(chip->convert_pin, 1);
+ gpiod_set_value(chip->convert_pin, 0);
} else { /* operating mode 1 */
- gpio_set_value(chip->convert_pin, 0);
- gpio_set_value(chip->convert_pin, 1);
+ gpiod_set_value(chip->convert_pin, 0);
+ gpiod_set_value(chip->convert_pin, 1);
}
- while (gpio_get_value(chip->busy_pin))
+ while (gpiod_get_value(chip->busy_pin))
cpu_relax();
- gpio_set_value(chip->rdwr_pin, 0);
- gpio_set_value(chip->rdwr_pin, 1);
+ gpiod_set_value(chip->rdwr_pin, 0);
+ gpiod_set_value(chip->rdwr_pin, 1);
ret = spi_read(spi_dev, , sizeof(*data));
if (ret < 0) {
dev_err(_dev->dev, "SPI data read error\n");
@@ -99,8 +99,8 @@ static int ad7816_spi_write(struct ad7816_chip_info *chip, u8
data)
struct spi_device *spi_dev = chip->spi_dev;
int ret = 0;
- gpio_set_value(chip->rdwr_pin, 1);
- gpio_set_value(chip->rdwr_pin, 0);
+ gpiod_set_value(chip->rdwr_pin, 1);
+ gpiod_set_value(chip->rdwr_pin, 0);
ret = spi_write(spi_dev, , sizeof(data));
if (ret < 0)
dev_err(_dev->dev, "SPI oti data write error\n");
@@ -129,10 +129,10 @@ static ssize_t ad7816_store_mode(struct device *dev,
struct ad7816_chip_info *chip = iio_priv(indio_dev);
if (strcmp(buf, "full")) {
- gpio_set_value(chip->rdwr_pin, 1);
+ gpiod_set_value(chip->rdwr_pin, 1);
chip->mode = AD7816_FULL;
} else {
- gpio_set_value(chip->rdwr_pin, 0);
+ gpiod_set_value(chip->rdwr_pin, 0);
chip->mode = AD7816_PD;
}
@@ -345,15 +345,9 @@ static int ad7816_probe(struct spi_device *spi_dev)
{
struct ad7816_chip_info *chip;
struct iio_dev *indio_dev;
- unsigned short *pins = dev_get_platdata(_dev->dev);
int ret = 0;
int i;
- if (!pins) {
- dev_err(_dev->dev, "No necessary GPIO platform data.\n");
- return -EINVAL;
- }
-
indio_dev = devm_iio_device_alloc(_dev->dev, sizeof(*chip));
if (!indio_dev)
return -ENOMEM;
@@ -364,34 +358,28 @@ static int ad7816_probe(struct spi_device *spi_dev)
chip->spi_dev = spi_dev;
for (i = 0; i <= AD7816_CS_MAX; i++)
chip->oti_data[i] = 203;
- chip->rdwr_pin = pins[0];
- chip->convert_pin = pins[1];
- chip->busy_pin = pins[2];
- ret = devm_gpio_request(_dev->dev, chip->rdwr_pin,
- spi_get_device_id(spi_dev)->name);
- if (ret) {
+ chip->rdwr_pin = devm_gpiod_get(_dev->dev, "rdwr", GPIOD_IN);
+ if (IS_ERR(chip->rdwr_pin)) {
+ ret = PTR_ERR(chip->rdwr_pin);
dev_err(_dev->dev, "Fail to request rdwr gpio PIN %d.\n",
- chip->rdwr_pin);
+ ret);
return ret;
}
- gpio_direction_input(chip->rdwr_pin);
- ret = devm_gpio_request(_dev->dev, chip->convert_pin,
- spi_get_device_id(spi_dev)->name);
- if (ret) {
+
Re: [PATCH v2] staging: add nrf24 driver
On Tue, Oct 16, 2018 at 02:41:50PM +0300, Dan Carpenter wrote:
> When we add drivers, can we use the new subsystem prefix for the driver?
> In other words:
>
> [PATCH] staging: nrf24: Add new driver for 2.4Ghz radio transceiver
>
Sure.
> This driver seems basically OK to me. I don't think you necessarily
> need to go through staging? Have you tried sending it directly to
> netdev?
>
I have not tried that, mainly as I believe it is not mature enough. I
would give it some time in staging for testing and bugfixing. If you say
that it is not needed and can be done out of staging, I will ask netdev
if it can be accepted there.
> > + //take out size of data
>
> This comment is sort of useless? "take out" is like Chinese food. It
> seems like it should be obvious what the code does. The format is
> wrong. So there are very minor things to be tidied up.
>
Agree, I will do comments cleanup.
> > + ret = kfifo_out(>tx_fifo, , sizeof(size));
> > + if (ret != sizeof(size)) {
> > + dev_dbg(>dev, "get size from fifo failed\n");
> > + mutex_unlock(>tx_fifo_mutex);
> > + continue;
> > + }
> > +
> > + //alloc space for data
> > + buf = kzalloc(size, GFP_KERNEL);
> > + if (!buf) {
> > + dev_dbg(>dev, "buf alloc failed\n");
> > + mutex_unlock(>tx_fifo_mutex);
> > + continue;
> > + }
> > +
> > + //take out size of data
> > + ret = kfifo_out(>tx_fifo, buf, size);
> > + if (ret != size) {
> > + dev_dbg(>dev, "get buf from fifo failed\n");
> > + mutex_unlock(>tx_fifo_mutex);
> > + goto next;
> > + }
> > +
> > + //unlock tx fifo
> > + mutex_unlock(>tx_fifo_mutex);
> > +
> > + //enter Standby-I mode
> > + nrf24_ce_lo(device);
> > +
> > + //set TX MODE
> > + ret = nrf24_set_mode(device->spi, NRF24_MODE_TX);
> > + if (ret < 0)
> > + goto next;
> > +
> > + //set PIPE0 address
> > + //this is needed to receive ACK
> > + ret = nrf24_set_address(device->spi,
> > + NRF24_PIPE0,
> > + (u8 *)>cfg.address);
> > + if (ret < 0) {
> > + dev_dbg(>dev, "set PIPE0 address failed
> > (%d)\n", ret);
> > + goto next;
> > + }
> > +
> > + //set TX address
> > + ret = nrf24_set_address(device->spi,
> > + NRF24_TX,
> > + (u8 *)>cfg.address);
> > + if (ret < 0) {
> > + dev_dbg(>dev, "set TX address failed (%d)\n",
> > ret);
> > + goto next;
> > + }
> > +
> > + //check if pipe uses static payload length
> > + spl = p->cfg.plw != 0;
> > +
> > + //check if dynamic payload length is enabled
> > + dpl = nrf24_get_dynamic_pl(device->spi);
> > +
> > + if (spl && dpl) {
> > + //disable dynamic payload if pipe
> > + //does not use dynamic payload
> > + //and dynamic paload is enabled
> > + ret = nrf24_disable_dynamic_pl(device->spi);
> > + if (ret < 0)
> > + goto next;
> > + }
> > +
> > + memset(pload, 0, PLOAD_MAX);
> > + memcpy(pload, , sizeof(size));
> > +
> > + //calculate payload length
> > + n = spl ? p->cfg.plw : sizeof(size);
> > +
> > + //send size
> > + nrf24_write_tx_pload(device->spi, pload, n);
> > + if (ret < 0) {
> > + dev_dbg(>dev, "write TX PLOAD failed (%d)\n",
> > ret);
> > + goto next;
> > + }
> > +
> > + //enter TX MODE and start transmission
> > + nrf24_ce_hi(device);
> > +
> > + //wait for ACK
> > + wait_event_interruptible(device->tx_done_wait_queue,
> > +(device->tx_done ||
> > +kthread_should_stop()));
> > +
> > + if (kthread_should_stop())
> > + goto abort;
> > +
> > + //clear counter
> > + sent = 0;
> > +
> > + while (size > 0) {
> > + n = spl ? p->cfg.plw : min_t(ssize_t, size, PLOAD_MAX);
> > +
> > + dev_dbg(>dev, "tx %zd bytes\n", n);
> > +
> > + memset(pload, 0, PLOAD_MAX);
> > + memcpy(pload, buf + sent, n);
> > +
> > + //write PLOAD to nRF FIFO
> > + ret = nrf24_write_tx_pload(device->spi, pload, n);
> > +
> > + if (ret < 0) {
> > + dev_dbg(>dev,
> > +
Re: [PATCH] staging: iio: adc: Add comments to prevent checks corrections
On Mon, Oct 15, 2018 at 06:25:21PM -0300, Gabriel Capella wrote: > This patch adds 3 comments in 2 different files. > These comments warn to don't correct the checks of type: > "CHECK: spaces preferred around that '-'" > > Signed-off-by: Gabriel Capella > --- > drivers/staging/iio/adc/ad7192.c | 1 + > drivers/staging/iio/adc/ad7280a.c | 2 ++ > 2 files changed, 3 insertions(+) > > diff --git a/drivers/staging/iio/adc/ad7192.c > b/drivers/staging/iio/adc/ad7192.c > index acdbc07fd259..786ace19c240 100644 > --- a/drivers/staging/iio/adc/ad7192.c > +++ b/drivers/staging/iio/adc/ad7192.c > @@ -354,6 +354,7 @@ ad7192_show_scale_available(struct device *dev, > return len; > } > > +/* Do not put spaces around the hyphen. #checkpatch.pl */ > static IIO_DEVICE_ATTR_NAMED(in_v_m_v_scale_available, >in_voltage-voltage_scale_available, >0444, ad7192_show_scale_available, NULL, 0); No one will read that... :( It has to be on the same line. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: gasket: remove debug logs in page table mapping calls
From: Todd Poynor
Remove very noisy debug logs that also contain typos and incorrect
output formats.
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/gasket_page_table.c | 24 --
1 file changed, 24 deletions(-)
diff --git a/drivers/staging/gasket/gasket_page_table.c
b/drivers/staging/gasket/gasket_page_table.c
index 5b398b7ba81d3..b7d460cf15fbc 100644
--- a/drivers/staging/gasket/gasket_page_table.c
+++ b/drivers/staging/gasket/gasket_page_table.c
@@ -477,7 +477,6 @@ static int gasket_perform_mapping(struct gasket_page_table
*pg_tbl,
for (i = 0; i < num_pages; i++) {
page_addr = host_addr + i * PAGE_SIZE;
offset = page_addr & (PAGE_SIZE - 1);
- dev_dbg(pg_tbl->device, "%s i %d\n", __func__, i);
if (is_coherent(pg_tbl, host_addr)) {
u64 off =
(u64)host_addr -
@@ -506,22 +505,9 @@ static int gasket_perform_mapping(struct gasket_page_table
*pg_tbl,
ptes[i].dma_addr =
dma_map_page(pg_tbl->device, page, 0, PAGE_SIZE,
DMA_BIDIRECTIONAL);
- dev_dbg(pg_tbl->device,
- "%s i %d pte %p pfn %p -> mapped %llx\n",
- __func__, i, [i],
- (void *)page_to_pfn(page),
- (unsigned long long)ptes[i].dma_addr);
if (dma_mapping_error(pg_tbl->device,
ptes[i].dma_addr)) {
- dev_dbg(pg_tbl->device,
- "%s i %d -> fail to map page %llx "
- "[pfn %p ohys %p]\n",
- __func__, i,
- (unsigned long long)ptes[i].dma_addr,
- (void *)page_to_pfn(page),
- (void *)page_to_phys(page));
-
if (gasket_release_page(ptes[i].page))
--pg_tbl->num_active_pages;
@@ -892,11 +878,6 @@ static int gasket_alloc_extended_subtable(struct
gasket_page_table *pg_tbl,
pte->dma_addr = dma_map_page(pg_tbl->device, pte->page, 0, PAGE_SIZE,
DMA_TO_DEVICE);
if (dma_mapping_error(pg_tbl->device, pte->dma_addr)) {
- dev_dbg(pg_tbl->device,
- "%s: fail to map page [pfn %lx phys %llx]\n",
- __func__, page_to_pfn(pte->page),
- page_to_phys(pte->page));
-
free_page(page_addr);
vfree(pte->sublevel);
memset(pte, 0, sizeof(struct gasket_page_table_entry));
@@ -1050,11 +1031,6 @@ int gasket_page_table_map(struct gasket_page_table
*pg_tbl, ulong host_addr,
}
mutex_unlock(_tbl->mutex);
-
- dev_dbg(pg_tbl->device,
- "%s done: ha %llx daddr %llx num %d, ret %d\n",
- __func__, (unsigned long long)host_addr,
- (unsigned long long)dev_addr, num_pages, ret);
return ret;
}
EXPORT_SYMBOL(gasket_page_table_map);
--
2.19.1.331.ge82ca0e54c-goog
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging: gasket: page_table: add mapping flags
From: Nick Ewalt
This allows for more precise dma_direction in the dma_map_page requests.
Also leaves room for adding more flags later.
Signed-off-by: Nick Ewalt
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/gasket.h| 33
drivers/staging/gasket/gasket_ioctl.c | 62 +++
drivers/staging/gasket/gasket_page_table.c | 87 ++
drivers/staging/gasket/gasket_page_table.h | 4 +-
4 files changed, 141 insertions(+), 45 deletions(-)
diff --git a/drivers/staging/gasket/gasket.h b/drivers/staging/gasket/gasket.h
index a0f065c517a52..93e7af1551975 100644
--- a/drivers/staging/gasket/gasket.h
+++ b/drivers/staging/gasket/gasket.h
@@ -37,6 +37,31 @@ struct gasket_page_table_ioctl {
u64 device_address;
};
+/*
+ * Structure for ioctl mapping buffers with flags when using the Gasket
+ * page_table module.
+ */
+struct gasket_page_table_ioctl_flags {
+ struct gasket_page_table_ioctl base;
+ /*
+* Flags indicating status and attribute requests from the host.
+* NOTE: STATUS bit does not need to be set in this request.
+* Set RESERVED bits to 0 to ensure backwards compatibility.
+*
+* Bitfields:
+* [0] - STATUS: indicates if this entry/slot is free
+*0 = PTE_FREE
+*1 = PTE_INUSE
+* [2:1] - DMA_DIRECTION: dma_data_direction requested by host
+* 00 = DMA_BIDIRECTIONAL
+* 01 = DMA_TO_DEVICE
+* 10 = DMA_FROM_DEVICE
+* 11 = DMA_NONE
+* [31:3] - RESERVED
+*/
+ u32 flags;
+};
+
/*
* Common structure for ioctls mapping and unmapping buffers when using the
* Gasket page_table module.
@@ -119,4 +144,12 @@ struct gasket_coherent_alloc_config_ioctl {
#define GASKET_IOCTL_CONFIG_COHERENT_ALLOCATOR
\
_IOWR(GASKET_IOCTL_BASE, 11, struct gasket_coherent_alloc_config_ioctl)
+/*
+ * Tells the kernel to map size bytes at host_address to device_address in
+ * page_table_index page table. Passes flags to indicate additional attribute
+ * requests for the mapped memory.
+ */
+#define GASKET_IOCTL_MAP_BUFFER_FLAGS
\
+ _IOW(GASKET_IOCTL_BASE, 12, struct gasket_page_table_ioctl_flags)
+
#endif /* __GASKET_H__ */
diff --git a/drivers/staging/gasket/gasket_ioctl.c
b/drivers/staging/gasket/gasket_ioctl.c
index 0ca48e688818f..e80f38509f47c 100644
--- a/drivers/staging/gasket/gasket_ioctl.c
+++ b/drivers/staging/gasket/gasket_ioctl.c
@@ -20,6 +20,7 @@
#define trace_gasket_ioctl_integer_data(x)
#define trace_gasket_ioctl_eventfd_data(x, ...)
#define trace_gasket_ioctl_page_table_data(x, ...)
+#define trace_gasket_ioctl_page_table_flags_data(x, ...)
#define trace_gasket_ioctl_config_coherent_allocator(x, ...)
#endif
@@ -130,29 +131,59 @@ static int gasket_partition_page_table(
}
/* Map a userspace buffer to a device virtual address. */
+static int gasket_map_buffers_common(
+ struct gasket_dev *gasket_dev,
+ struct gasket_page_table_ioctl_flags *pibuf)
+{
+ if (pibuf->base.page_table_index >= gasket_dev->num_page_tables)
+ return -EFAULT;
+
+ if
(gasket_page_table_are_addrs_bad(gasket_dev->page_table[pibuf->base.page_table_index],
+ pibuf->base.host_address,
+ pibuf->base.device_address,
+ pibuf->base.size))
+ return -EINVAL;
+
+ return
gasket_page_table_map(gasket_dev->page_table[pibuf->base.page_table_index],
+pibuf->base.host_address,
+pibuf->base.device_address,
+pibuf->base.size / PAGE_SIZE,
+pibuf->flags);
+}
+
static int gasket_map_buffers(struct gasket_dev *gasket_dev,
struct gasket_page_table_ioctl __user *argp)
{
- struct gasket_page_table_ioctl ibuf;
+ struct gasket_page_table_ioctl_flags ibuf;
- if (copy_from_user(, argp, sizeof(struct gasket_page_table_ioctl)))
+ if (copy_from_user(, argp, sizeof(struct
gasket_page_table_ioctl)))
return -EFAULT;
- trace_gasket_ioctl_page_table_data(ibuf.page_table_index, ibuf.size,
- ibuf.host_address,
- ibuf.device_address);
+ ibuf.flags = 0;
- if (ibuf.page_table_index >= gasket_dev->num_page_tables)
+ trace_gasket_ioctl_page_table_data(ibuf.base.page_table_index,
+ ibuf.base.size,
+ ibuf.base.host_address,
+ ibuf.base.device_address);
+
+ return
[PATCH 0/3] staging: gasket: add DMA direction flags a.o.
From: Todd Poynor Add flags to page mapping ioctls that specify DMA directions other than bi-directional, avoiding unnecessary cache maintenance for read-only or write-only buffers. Remove some spammy / unhelpful / inaccurately formatted debug logs. Nick Ewalt (1): staging: gasket: page_table: add mapping flags Todd Poynor (2): staging: gasket: remove debug logs for callback invocation staging: gasket: remove debug logs in page table mapping calls drivers/staging/gasket/gasket.h| 33 ++ drivers/staging/gasket/gasket_core.c | 7 +- drivers/staging/gasket/gasket_ioctl.c | 62 +--- drivers/staging/gasket/gasket_page_table.c | 111 +++-- drivers/staging/gasket/gasket_page_table.h | 4 +- 5 files changed, 142 insertions(+), 75 deletions(-) -- 2.19.1.331.ge82ca0e54c-goog ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: gasket: remove debug logs for callback invocation
From: Todd Poynor
Debug logs for device-specific callback invocation aren't very useful,
remove.
Signed-off-by: Todd Poynor
---
drivers/staging/gasket/gasket_core.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/staging/gasket/gasket_core.c
b/drivers/staging/gasket/gasket_core.c
index f230bec76ae4e..a445d58fb3999 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -109,8 +109,6 @@ check_and_invoke_callback(struct gasket_dev *gasket_dev,
{
int ret = 0;
- dev_dbg(gasket_dev->dev, "check_and_invoke_callback %p\n",
- cb_function);
if (cb_function) {
mutex_lock(_dev->mutex);
ret = cb_function(gasket_dev);
@@ -126,11 +124,8 @@ gasket_check_and_invoke_callback_nolock(struct gasket_dev
*gasket_dev,
{
int ret = 0;
- if (cb_function) {
- dev_dbg(gasket_dev->dev,
- "Invoking device-specific callback.\n");
+ if (cb_function)
ret = cb_function(gasket_dev);
- }
return ret;
}
--
2.19.1.331.ge82ca0e54c-goog
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging : Add RIFFA PCIe driver
Please make the subject be: [PATCH] Staging: riffa: Add new RIFFA PCIe driver On Tue, Oct 16, 2018 at 10:54:28AM +, Cheng Fei Phung wrote: > This patch adds RIFFA PCIe linux driver for > https://github.com/promach/riffa/tree/full_duplex/driver/linux Line wrap the commit message at 75 characters or less. > > TODO: > > 1) this driver needs further speed optimization although it can now achieve > defined PCIe speed grade > 2) solve all the coding style errors from scripts/checkpatch.pl Greg means that it needs a file named drivers/staging/riffa/TODO > > Signed-off-by: PHUNG CHENG FEI (feiph...@hotmail.com) Please use: Signed-off-by: Cheng Fei Phung It's really unusual for any driver to not use the kernel build system but to come with its own makefile like an out of tree driver... I don't care too much about that because this is staging and we will fix it, but I had wanted to read the Kconfig entry but it's not there... Anyway, this is a staging drive so we're not picky at all. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: add nrf24 driver
On Tue, 2018-10-16 at 01:17 +0200, Marcin Ciupak wrote:
> This patch adds driver for Nordic Semiconductor nRF24L01+ radio module.
[]
> diff --git a/drivers/staging/nrf24/nrf24_hal.c
> b/drivers/staging/nrf24/nrf24_hal.c
[]
> +static ssize_t nrf24_read_reg(struct spi_device *spi, u8 addr)
> +{
> + ssize_t ret;
> +
> + ret = spi_w8r8(spi, addr);
> +
> + if (ret < 0)
> + dev_dbg(>dev, "%s: read 0x%X FAILED", __func__, addr);
Please add terminating newlines to all logging formats
dev_dbg(>dev, "%s: read 0x%X FAILED\n", __func__, addr);
etc...
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: add nrf24 driver
When we add drivers, can we use the new subsystem prefix for the driver?
In other words:
[PATCH] staging: nrf24: Add new driver for 2.4Ghz radio transceiver
This driver seems basically OK to me. I don't think you necessarily
need to go through staging? Have you tried sending it directly to
netdev?
> + //take out size of data
This comment is sort of useless? "take out" is like Chinese food. It
seems like it should be obvious what the code does. The format is
wrong. So there are very minor things to be tidied up.
> + ret = kfifo_out(>tx_fifo, , sizeof(size));
> + if (ret != sizeof(size)) {
> + dev_dbg(>dev, "get size from fifo failed\n");
> + mutex_unlock(>tx_fifo_mutex);
> + continue;
> + }
> +
> + //alloc space for data
> + buf = kzalloc(size, GFP_KERNEL);
> + if (!buf) {
> + dev_dbg(>dev, "buf alloc failed\n");
> + mutex_unlock(>tx_fifo_mutex);
> + continue;
> + }
> +
> + //take out size of data
> + ret = kfifo_out(>tx_fifo, buf, size);
> + if (ret != size) {
> + dev_dbg(>dev, "get buf from fifo failed\n");
> + mutex_unlock(>tx_fifo_mutex);
> + goto next;
> + }
> +
> + //unlock tx fifo
> + mutex_unlock(>tx_fifo_mutex);
> +
> + //enter Standby-I mode
> + nrf24_ce_lo(device);
> +
> + //set TX MODE
> + ret = nrf24_set_mode(device->spi, NRF24_MODE_TX);
> + if (ret < 0)
> + goto next;
> +
> + //set PIPE0 address
> + //this is needed to receive ACK
> + ret = nrf24_set_address(device->spi,
> + NRF24_PIPE0,
> + (u8 *)>cfg.address);
> + if (ret < 0) {
> + dev_dbg(>dev, "set PIPE0 address failed
> (%d)\n", ret);
> + goto next;
> + }
> +
> + //set TX address
> + ret = nrf24_set_address(device->spi,
> + NRF24_TX,
> + (u8 *)>cfg.address);
> + if (ret < 0) {
> + dev_dbg(>dev, "set TX address failed (%d)\n",
> ret);
> + goto next;
> + }
> +
> + //check if pipe uses static payload length
> + spl = p->cfg.plw != 0;
> +
> + //check if dynamic payload length is enabled
> + dpl = nrf24_get_dynamic_pl(device->spi);
> +
> + if (spl && dpl) {
> + //disable dynamic payload if pipe
> + //does not use dynamic payload
> + //and dynamic paload is enabled
> + ret = nrf24_disable_dynamic_pl(device->spi);
> + if (ret < 0)
> + goto next;
> + }
> +
> + memset(pload, 0, PLOAD_MAX);
> + memcpy(pload, , sizeof(size));
> +
> + //calculate payload length
> + n = spl ? p->cfg.plw : sizeof(size);
> +
> + //send size
> + nrf24_write_tx_pload(device->spi, pload, n);
> + if (ret < 0) {
> + dev_dbg(>dev, "write TX PLOAD failed (%d)\n",
> ret);
> + goto next;
> + }
> +
> + //enter TX MODE and start transmission
> + nrf24_ce_hi(device);
> +
> + //wait for ACK
> + wait_event_interruptible(device->tx_done_wait_queue,
> + (device->tx_done ||
> + kthread_should_stop()));
> +
> + if (kthread_should_stop())
> + goto abort;
> +
> + //clear counter
> + sent = 0;
> +
> + while (size > 0) {
> + n = spl ? p->cfg.plw : min_t(ssize_t, size, PLOAD_MAX);
> +
> + dev_dbg(>dev, "tx %zd bytes\n", n);
> +
> + memset(pload, 0, PLOAD_MAX);
> + memcpy(pload, buf + sent, n);
> +
> + //write PLOAD to nRF FIFO
> + ret = nrf24_write_tx_pload(device->spi, pload, n);
> +
> + if (ret < 0) {
> + dev_dbg(>dev,
> + "write TX PLOAD failed (%d)\n",
> + ret);
> + goto next;
> + }
> +
> + sent += n;
> + size -= n;
> +
> + device->tx_done = false;
> +
> + //wait for ACK
> + wait_event_interruptible(device->tx_done_wait_queue,
> +
Re: [PATCH] staging: erofs: add the missing __init tags
On 2018/10/9 21:43, Gao Xiang wrote: > Append __init to `erofs_init_inode_cache', > `z_erofs_init_zip_subsystem' and move these > declarations to internal.h. > > Signed-off-by: Gao Xiang Reviewed-by: Chao Yu Thanks, ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging : Add RIFFA PCIe driver
On Tue, Oct 16, 2018 at 10:54:28AM +, Cheng Fei Phung wrote:
> This patch adds RIFFA PCIe linux driver for
> https://github.com/promach/riffa/tree/full_duplex/driver/linux
>
> TODO:
>
> 1) this driver needs further speed optimization although it can now achieve
> defined PCIe speed grade
> 2) solve all the coding style errors from scripts/checkpatch.pl
Can you please wrap your lines at 72 columns like your editor wanted you
to? :)
>
> Signed-off-by: PHUNG CHENG FEI (feiph...@hotmail.com)
So close, you need to use '<' and '>' here, not '(' and ')'
>
> ---
> Changes in v1:
> - added full-duplex capability
>
> drivers/staging/riffa/Makefile | 157 +++
> drivers/staging/riffa/README.txt | 38 +
> drivers/staging/riffa/circ_queue.c | 188 +++
> drivers/staging/riffa/circ_queue.h | 96 ++
> drivers/staging/riffa/riffa.c | 152 +++
> drivers/staging/riffa/riffa.h | 121 ++
> drivers/staging/riffa/riffa_driver.c | 1633 ++
> drivers/staging/riffa/riffa_driver.h | 131 +++
> 8 files changed, 2516 insertions(+)
> create mode 100644 drivers/staging/riffa/Makefile
> create mode 100644 drivers/staging/riffa/README.txt
> create mode 100644 drivers/staging/riffa/circ_queue.c
> create mode 100644 drivers/staging/riffa/circ_queue.h
> create mode 100644 drivers/staging/riffa/riffa.c
> create mode 100644 drivers/staging/riffa/riffa.h
> create mode 100644 drivers/staging/riffa/riffa_driver.c
> create mode 100644 drivers/staging/riffa/riffa_driver.h
This does not actually add the driver to the kernel build process at
all. How are you testing this? You need to hook it up by adding a
Kconfig file and touching the drivers/staging/Kconfig and
drivers/staging/Makefile files to add your code to the build system.
As it is, this doesn't work at all :(
And none of your code has any tabs in it at all. You should at least
fix that up first, otherwise the very first patch people are going to
be forced to send will be to replace all lines in all of these files
with that type of modification. It is better if you do that first.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Add RIFFA PCIe driver to staging tree
On Tue, Oct 16, 2018 at 07:41:45AM +, Cheng Fei Phung wrote: > Patch changelog content: > > Add a PCIe linux driver called RIFFA to the linux staging tree. It has full > duplex capability compared to the original RIFFA linux driver that you can > easily find from https://github.com/KastnerRG/riffa/tree/master/driver/linux > > For more context, please refer to > https://github.com/promach/riffa/tree/full_duplex/driver/linux > > > > phung@UbuntuHW15:~/linux$ cat > patches/0001-Added-a-full-duplex-riffa-PCIe-linux-staging-driver..patch > >From 7c3050c7e86017d85c75c640ea38376f505b2763 Mon Sep 17 00:00:00 2001 > From: promach > Date: Mon, 15 Oct 2018 18:28:16 +0800 > Subject: [PATCH] Added a full duplex riffa PCIe linux staging driver. > Signed-off-by: PHUNG CHENG FEI (feiph...@hotmail.com) > This whole format is very odd. Please look at how other patches are posted to the list. I need a proper changelog and signed-off-by line in the correct place. Also, no need to 'cat' the file here, just use git send-email to send the patch correctly. Because of this, all of your tabs got converted to spaces, so the patch you sent here is corrupted. Try sending it to yourself first, run the result of that email through scripts/checkpatch.pl and apply it to the tree to verify that it really does work. Also, for staging drivers I need a TODO file that lists the remaining tasks that are left to be done in order to get the driver out of the staging portion of the kernel tree. Look at the patch that was sent today that added a new driver to the staging portion of the kernel as an example of how to do all of this correctly. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: add nrf24 driver
This patch adds driver for Nordic Semiconductor nRF24L01+ radio module.
Signed-off-by: Marcin Ciupak
---
Changes in v2:
- add terminating newlines to all logging formats
drivers/staging/Kconfig | 2 +
drivers/staging/Makefile | 1 +
drivers/staging/nrf24/Kconfig | 16 +
drivers/staging/nrf24/Makefile| 3 +
drivers/staging/nrf24/TODO| 7 +
.../nrf24/devicetree/nrf24-spi0-overlay.dts | 54 +
.../nrf24/devicetree/nrf24-spi1-overlay.dts | 54 +
drivers/staging/nrf24/devicetree/nrf24.txt| 1 +
drivers/staging/nrf24/nRF24L01.h | 82 ++
drivers/staging/nrf24/nrf24_enums.h | 60 ++
drivers/staging/nrf24/nrf24_hal.c | 764 +++
drivers/staging/nrf24/nrf24_hal.h | 54 +
drivers/staging/nrf24/nrf24_if.c | 919 ++
drivers/staging/nrf24/nrf24_if.h | 63 ++
drivers/staging/nrf24/nrf24_sysfs.c | 707 ++
drivers/staging/nrf24/nrf24_sysfs.h | 14 +
16 files changed, 2801 insertions(+)
create mode 100644 drivers/staging/nrf24/Kconfig
create mode 100644 drivers/staging/nrf24/Makefile
create mode 100644 drivers/staging/nrf24/TODO
create mode 100644 drivers/staging/nrf24/devicetree/nrf24-spi0-overlay.dts
create mode 100644 drivers/staging/nrf24/devicetree/nrf24-spi1-overlay.dts
create mode 100644 drivers/staging/nrf24/devicetree/nrf24.txt
create mode 100644 drivers/staging/nrf24/nRF24L01.h
create mode 100644 drivers/staging/nrf24/nrf24_enums.h
create mode 100644 drivers/staging/nrf24/nrf24_hal.c
create mode 100644 drivers/staging/nrf24/nrf24_hal.h
create mode 100644 drivers/staging/nrf24/nrf24_if.c
create mode 100644 drivers/staging/nrf24/nrf24_if.h
create mode 100644 drivers/staging/nrf24/nrf24_sysfs.c
create mode 100644 drivers/staging/nrf24/nrf24_sysfs.h
diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 1abf76be2aa8..55d688f3112e 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -126,4 +126,6 @@ source "drivers/staging/axis-fifo/Kconfig"
source "drivers/staging/erofs/Kconfig"
+source "drivers/staging/nrf24/Kconfig"
+
endif # STAGING
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index ab0cbe8815b1..c18e74df03af 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -53,3 +53,4 @@ obj-$(CONFIG_SOC_MT7621) += mt7621-dts/
obj-$(CONFIG_STAGING_GASKET_FRAMEWORK) += gasket/
obj-$(CONFIG_XIL_AXIS_FIFO)+= axis-fifo/
obj-$(CONFIG_EROFS_FS) += erofs/
+obj-$(CONFIG_NRF24)+= nrf24/
diff --git a/drivers/staging/nrf24/Kconfig b/drivers/staging/nrf24/Kconfig
new file mode 100644
index ..67ebf14dd982
--- /dev/null
+++ b/drivers/staging/nrf24/Kconfig
@@ -0,0 +1,16 @@
+config NRF24
+tristate "nRF24L01+ 2.4GHz radio module support"
+depends on SPI
+help
+ This enables support for Nordic Semiconductor nRF24L01+ radio module,
+ with the following features:
+- multiple radio module instances via nrfX
+- dedicated /dev/nrfX.Y device per pipe per instance
+- dynamic and static payload lengths
+- configuration via sysfs (/sys/class/nrfX)
+- poll mechanism
+- 64kB RX FIFO per pipe
+- 64kB TX FIFO
+
+ To compile this driver as a module, choose M here: the module will be
+ called nrf24.
diff --git a/drivers/staging/nrf24/Makefile b/drivers/staging/nrf24/Makefile
new file mode 100644
index ..f5222567c632
--- /dev/null
+++ b/drivers/staging/nrf24/Makefile
@@ -0,0 +1,3 @@
+obj-$(CONFIG_NRF24) += nrf24.o
+
+nrf24-objs := nrf24_if.o nrf24_hal.o nrf24_sysfs.o
diff --git a/drivers/staging/nrf24/TODO b/drivers/staging/nrf24/TODO
new file mode 100644
index ..a089e43faac5
--- /dev/null
+++ b/drivers/staging/nrf24/TODO
@@ -0,0 +1,7 @@
+Todo:
+- opening and closing pipes via sysfs
+- improve switching in between RX and TX
+- improve handling of MAX_RT interrupt
+- find and fix bugs
+- code cleanup
+
diff --git a/drivers/staging/nrf24/devicetree/nrf24-spi0-overlay.dts
b/drivers/staging/nrf24/devicetree/nrf24-spi0-overlay.dts
new file mode 100644
index ..130e6787b76d
--- /dev/null
+++ b/drivers/staging/nrf24/devicetree/nrf24-spi0-overlay.dts
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//
+// Copyright (C) 2017 Marcin Ciupak
+//
+
+// Definitions for NRF24
+/dts-v1/;
+/plugin/;
+
+/ {
+ compatible = "bcm,bcm2835", "bcm,bcm2708", "bcm,bcm2709";
+
+ fragment@0 {
+ target = <>;
+ __overlay__ {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "okay";
+
+ spidev@0 {
+ status = "disabled";
+ };
+
+
[PATCH] Add RIFFA PCIe driver to staging tree
Patch changelog content: Add a PCIe linux driver called RIFFA to the linux staging tree. It has full duplex capability compared to the original RIFFA linux driver that you can easily find from https://github.com/KastnerRG/riffa/tree/master/driver/linux For more context, please refer to https://github.com/promach/riffa/tree/full_duplex/driver/linux phung@UbuntuHW15:~/linux$ cat patches/0001-Added-a-full-duplex-riffa-PCIe-linux-staging-driver..patch >From 7c3050c7e86017d85c75c640ea38376f505b2763 Mon Sep 17 00:00:00 2001 From: promach Date: Mon, 15 Oct 2018 18:28:16 +0800 Subject: [PATCH] Added a full duplex riffa PCIe linux staging driver. Signed-off-by: PHUNG CHENG FEI (feiph...@hotmail.com) --- drivers/staging/riffa/Makefile | 157 +++ drivers/staging/riffa/README.txt | 38 + drivers/staging/riffa/circ_queue.c | 188 +++ drivers/staging/riffa/circ_queue.h | 96 ++ drivers/staging/riffa/riffa.c | 152 +++ drivers/staging/riffa/riffa.h | 121 ++ drivers/staging/riffa/riffa_driver.c | 1633 ++ drivers/staging/riffa/riffa_driver.h | 131 +++ 8 files changed, 2516 insertions(+) create mode 100644 drivers/staging/riffa/Makefile create mode 100644 drivers/staging/riffa/README.txt create mode 100644 drivers/staging/riffa/circ_queue.c create mode 100644 drivers/staging/riffa/circ_queue.h create mode 100644 drivers/staging/riffa/riffa.c create mode 100644 drivers/staging/riffa/riffa.h create mode 100644 drivers/staging/riffa/riffa_driver.c create mode 100644 drivers/staging/riffa/riffa_driver.h diff --git a/drivers/staging/riffa/Makefile b/drivers/staging/riffa/Makefile new file mode 100644 index ..3e3cb0c4a387 --- /dev/null +++ b/drivers/staging/riffa/Makefile @@ -0,0 +1,157 @@ +# -- +# Copyright (c) 2016, The Regents of the University of California All +# rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following +# disclaimer in the documentation and/or other materials provided +# with the distribution. +# +# * Neither the name of The Regents of the University of California +# nor the names of its contributors may be used to endorse or +# promote products derived from this software without specific +# prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL REGENTS OF THE +# UNIVERSITY OF CALIFORNIA BE LIABLE FOR ANY DIRECT, INDIRECT, +# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, +# BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS +# OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND +# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR +# TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE +# USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH +# DAMAGE. +# -- + +# Filename: Makefile +# Version: 2.0 +# Description: Makefile for Linux PCIe device driver for RIFFA. +# Author: Matthew Jacobsen +# History: @mattj: Initial release. Version 2.0. + +# You must specify the following variables. You can leave the defaults if you +# like, but make sure they will work in your system. +# The VENDOR_ID _must_ match what is configured on your FPGA's PCIe endpoint +# header. Xilinx has a VENDOR_ID = 10EE. +NAME := riffa +VENDOR_ID0 := 10EE +VENDOR_ID1 := 1172 +MAJNUM := 100 + +# Build variables +KVER := $(shell uname -r) +KDIR := /lib/modules/`uname -r`/build +RHR := /etc/redhat-release +LIB_SRCS := riffa.c +LIB_OBJS := $(patsubst %.c,%.o,$(LIB_SRCS)) +LIB_HDR := riffa.h +LIB_VER_MAJ := 1 +LIB_VER_MIN := 0 +LIB_VER := $(LIB_VER_MAJ).$(LIB_VER_MIN) +DRVR_HDR := riffa_driver.h +DBUGVAL := DBUG + +obj-m += $(NAME).o +$(NAME)-y := riffa_driver.o circ_queue.o + +# Helper functions +define assert + $(if $1,,$(error Assertion failed: $2)) +endef +define assert-not-null + $(call assert,$($1),The variable "$1" is null, please specify it.) +endef +define assert-variables + $(call assert-not-null,NAME) + $(call assert-not-null,MAJNUM) + $(call assert-not-null,VENDOR_ID0) + $(call assert-not-null,VENDOR_ID1) + @printf "Compiling driver for kernel: %s with the following values\n" $(KVER) + @printf " NAME: '%s'\n"
