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-kernel@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
[v5 3/4] mpt3sas:Fix driver modifying persistent data.
* If EEDPTagMode field in manufacturing page11 is set,
unset it. This is needed to fix a hardware bug
in SAS3/SAS2 cards, So, skipping EEDPTagMode changes
in Manufacturing page11 for SAS35 controllers.
* Fix driver modifying NVRAM/persistent data in
Manufacturing page11 along with current copy. Driver should
change only current copy of Manufacturing page11.
Signed-off-by: Suganath Prabu
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 2 +-
drivers/scsi/mpt3sas/mpt3sas_config.c | 4
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index a6c217c..770f52b 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -4062,7 +4062,7 @@ _base_static_config_pages(struct MPT3SAS_ADAPTER *ioc)
* flag unset in NVDATA.
*/
mpt3sas_config_get_manufacturing_pg11(ioc, _reply, >manu_pg11);
- if (ioc->manu_pg11.EEDPTagMode == 0) {
+ if ((!ioc->is_gen35_ioc) && (ioc->manu_pg11.EEDPTagMode == 0)) {
pr_err("%s: overriding NVDATA EEDPTagMode setting\n",
ioc->name);
ioc->manu_pg11.EEDPTagMode &= ~0x3;
diff --git a/drivers/scsi/mpt3sas/mpt3sas_config.c
b/drivers/scsi/mpt3sas/mpt3sas_config.c
index 14a195c..aa41cb9 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_config.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_config.c
@@ -660,10 +660,6 @@ mpt3sas_config_set_manufacturing_pg11(struct
MPT3SAS_ADAPTER *ioc,
r = _config_request(ioc, _request, mpi_reply,
MPT3_CONFIG_PAGE_DEFAULT_TIMEOUT, config_page,
sizeof(*config_page));
- mpi_request.Action = MPI2_CONFIG_ACTION_PAGE_WRITE_NVRAM;
- r = _config_request(ioc, _request, mpi_reply,
- MPT3_CONFIG_PAGE_DEFAULT_TIMEOUT, config_page,
- sizeof(*config_page));
out:
return r;
}
--
1.8.3.1
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-kernel@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
[v5 3/4] mpt3sas:Fix driver modifying persistent data.
* If EEDPTagMode field in manufacturing page11 is set,
unset it. This is needed to fix a hardware bug
in SAS3/SAS2 cards, So, skipping EEDPTagMode changes
in Manufacturing page11 for SAS35 controllers.
* Fix driver modifying NVRAM/persistent data in
Manufacturing page11 along with current copy. Driver should
change only current copy of Manufacturing page11.
Signed-off-by: Suganath Prabu
---
drivers/scsi/mpt3sas/mpt3sas_base.c | 2 +-
drivers/scsi/mpt3sas/mpt3sas_config.c | 4
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index a6c217c..770f52b 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -4062,7 +4062,7 @@ _base_static_config_pages(struct MPT3SAS_ADAPTER *ioc)
* flag unset in NVDATA.
*/
mpt3sas_config_get_manufacturing_pg11(ioc, _reply, >manu_pg11);
- if (ioc->manu_pg11.EEDPTagMode == 0) {
+ if ((!ioc->is_gen35_ioc) && (ioc->manu_pg11.EEDPTagMode == 0)) {
pr_err("%s: overriding NVDATA EEDPTagMode setting\n",
ioc->name);
ioc->manu_pg11.EEDPTagMode &= ~0x3;
diff --git a/drivers/scsi/mpt3sas/mpt3sas_config.c
b/drivers/scsi/mpt3sas/mpt3sas_config.c
index 14a195c..aa41cb9 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_config.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_config.c
@@ -660,10 +660,6 @@ mpt3sas_config_set_manufacturing_pg11(struct
MPT3SAS_ADAPTER *ioc,
r = _config_request(ioc, _request, mpi_reply,
MPT3_CONFIG_PAGE_DEFAULT_TIMEOUT, config_page,
sizeof(*config_page));
- mpi_request.Action = MPI2_CONFIG_ACTION_PAGE_WRITE_NVRAM;
- r = _config_request(ioc, _request, mpi_reply,
- MPT3_CONFIG_PAGE_DEFAULT_TIMEOUT, config_page,
- sizeof(*config_page));
out:
return r;
}
--
1.8.3.1
Fate of ihex2fw tool
Everyone:
Since commit 5620a0d1aacd554ebebcff373e31107bb1ef7769 ("firmware:
delete in-kernel firmware"):
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v4.19-rc7=5620a0d1aacd554ebebcff373e31107bb1ef7769
firmware/ihex2fw.c is gone from the source tree and with it ihex2fw
tool used to generate sruct ihxed_binrec {} filled images (.ihex ->
.fw) consumed by request_ihex_frimware().
I tried looking for that too in
https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
but couldn't find it there.
Is there a new/different source tree it can be found nowadays?
Or is there another established tool that can do .ihex -> .fw
conversion that is expected to be used instead of ihex2bin?
Thanks,
Andrey Smirnov
Fate of ihex2fw tool
Everyone:
Since commit 5620a0d1aacd554ebebcff373e31107bb1ef7769 ("firmware:
delete in-kernel firmware"):
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v4.19-rc7=5620a0d1aacd554ebebcff373e31107bb1ef7769
firmware/ihex2fw.c is gone from the source tree and with it ihex2fw
tool used to generate sruct ihxed_binrec {} filled images (.ihex ->
.fw) consumed by request_ihex_frimware().
I tried looking for that too in
https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git
but couldn't find it there.
Is there a new/different source tree it can be found nowadays?
Or is there another established tool that can do .ihex -> .fw
conversion that is expected to be used instead of ihex2bin?
Thanks,
Andrey Smirnov
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
>
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
>
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?
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
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?
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
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
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
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
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
Re: [PATCH v2 6/6] arm64: platform: Add enable Exynos Multi-Core Timer driver
Hi Marek, On 2018년 10월 15일 21:31, Marek Szyprowski wrote: > On Exynos SoCs enabling MCT driver is required even if ARM Architected > Timer driver is used to for managing timer hardware and clock source > events. > > Signed-off-by: Marek Szyprowski > --- > arch/arm64/Kconfig.platforms | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms > index 51bc479334a4..7cc687580fad 100644 > --- a/arch/arm64/Kconfig.platforms > +++ b/arch/arm64/Kconfig.platforms > @@ -62,6 +62,7 @@ config ARCH_BRCMSTB > config ARCH_EXYNOS > bool "ARMv8 based Samsung Exynos SoC family" > select COMMON_CLK_SAMSUNG > + select CLKSRC_EXYNOS_MCT > select EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS > select EXYNOS_PMU > select HAVE_S3C2410_WATCHDOG if WATCHDOG > I tested it on Exynos5433-based TM2 board. Reviewed-by: Chanwoo Choi Tested-by: Chanwoo Choi -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH] Kbuild: Hide Clang's -Wempty-body behind W=1
Hi Masahiro,
On Wed, Oct 17, 2018 at 01:48:46PM +0900, Masahiro Yamada wrote:
> Hi Nathan,
>
>
> On Tue, Oct 16, 2018 at 11:15 AM Nathan Chancellor
> wrote:
> >
> > There are only a few instances of this warning in an arm64 allyesconfig
> > build but none of them appear useful. I believe the intention of the
> > warning is to avoid situations like this:
> >
> > if (condition);
> > statement;
> >
> > where the user really intended
> >
> > if (condition)
> > statement;
> >
> > However, these instances have already been caught by GCC's warning about
> > misleading indentation
>
>
> Right, the example above is caught by -Wmisleading-indentation.
>
> However, the following is not.
>
>if (condition)
> ;
>
>
>
> So, -Wempty-body is a kind of different thing,
> and still useful in my opinion.
>
>
>
> > so the remaining warnings are about loops that
> > fall into one of three categories:
> >
> > 1. Execute a function unconditionally (avoiding a useless variable to
> >hold the return value):
> >
> > drivers/isdn/hisax/hfc_pci.c:131:34: warning: if statement has empty body
> > [-Wempty-body]
> > if (Read_hfc(cs, HFCPCI_INT_S1));
>
>
> I think this is a real bug,
> then -Wempty-body finally caught it.
> (but -Wmisleading-indentation cannot catch it.)
>
>
>
> It is wrong to enclose a non-effective statement with 'if ();'
> just for suppressing another warning.
>
>
>
> Read_hfc(cs, HFCPCI_INT_S1);
>
> would emit this warning.
>
>
> In file included from drivers/isdn/hisax/hfc_pci.c:20:0:
> drivers/isdn/hisax/hfc_pci.c: In function ‘reset_hfcpci’:
> drivers/isdn/hisax/hfc_pci.h:232:25: warning: statement with no effect
> [-Wunused-value]
> #define Read_hfc(a, b) (*(((u_char *)a->hw.hfcpci.pci_io) + b))
> ~^~~
> drivers/isdn/hisax/hfc_pci.c:131:2: note: in expansion of macro ‘Read_hfc’
> Read_hfc(cs, HFCPCI_INT_S1);
> ^~~~
>
>
>
> The root cause is missing 'volatile'
> while Read_hfc() is supposed to read out a HW register.
>
>
>
> #define Read_hfc(a, b) (*(((volatile u_char *)a->hw.hfcpci.pci_io) + b))
>
> will be a correct fix.
> (or just use a standard accessor like readb(), ioread8(), etc.)
>
>
>
>
> if (Read_hfc(cs, HFCPCI_INT_S1));
>
> is optimized out by the compiler, so it is not working as expected.
>
>
>
> >
> > 2. Advancing a value to be used later on in the function like a pointer
> >or a count:
> >
> > drivers/atm/eni.c:244:48: warning: for loop has empty body
> > [-Wempty-body]
> > for (order = 0; (1 << order) < *size; order++);
> > ^
>
> As you noted in the commit log,
> Clang's -Wempty-body cares the location of a semi-colon,
> while GCC's one does not.
>
>
>
>
>
>for (order = 0; (1 << order) < *size; order++)
> ;
>
> is fine, and more readable in my opinion.
>
>
>
>
> > 3. Busy waiting:
> >
> > drivers/atm/zatm.c:513:7: warning: while loop has empty body
> > [-Wempty-body]
> > zwait;
> > ^
>
>
> Again, Clang is fine with an empty body in while() loop,
> but just picky about the semi-colon location.
>
> For this particular case, how about something like this?
>
>
> #define zwait do {} while (zin(CMR) & uPD98401_BUSY)
>
>
>
>
>
> I think an even better fix is
>
> #define zwait() do {} while (zin(CMR) & uPD98401_BUSY)
>
>
>
> then, fix-up all
>
>zwait;
>
> to
>
>zwait();
>
>
>
>
>
>
> > None of these uses are problematic or need to be addressed.
>
>
> The first pattern is really problematic, and need to be addressed.
>
> I want to keep -Wempty-body enabled
> to find out potential issues.
>
> Please let me know if you see other patterns difficult to fix.
>
>
>
Thank you very much for the quick feedback, this all sounds reasonable.
I will go ahead and dig into these further and send out patches to
address them.
Much appreciated,
Nathan
>
>
> > Clang
> > suggests moving the semi-colon to the next line to silence these
> > warnings but that defeats the purpose of the compact nature of these
> > constructs so just hide the warning behind W=1 so its use can still be
> > audited but it won't polute a regular build.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/42
> > Link: https://github.com/ClangBuiltLinux/linux/issues/66
> > Signed-off-by: Nathan Chancellor
> > ---
> > scripts/Makefile.extrawarn | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> > index cf6cd0ef6975..8709d9d6faf1 100644
> > --- a/scripts/Makefile.extrawarn
> > +++ b/scripts/Makefile.extrawarn
> > @@ -11,6 +11,7 @@
> > # are not supported by all versions of the compiler
> > #
> > ==
> >
> > +KBUILD_CFLAGS += $(call cc-disable-warning, empty-body)
> > KBUILD_CFLAGS += $(call
Re: [PATCH v2 6/6] arm64: platform: Add enable Exynos Multi-Core Timer driver
Hi Marek, On 2018년 10월 15일 21:31, Marek Szyprowski wrote: > On Exynos SoCs enabling MCT driver is required even if ARM Architected > Timer driver is used to for managing timer hardware and clock source > events. > > Signed-off-by: Marek Szyprowski > --- > arch/arm64/Kconfig.platforms | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms > index 51bc479334a4..7cc687580fad 100644 > --- a/arch/arm64/Kconfig.platforms > +++ b/arch/arm64/Kconfig.platforms > @@ -62,6 +62,7 @@ config ARCH_BRCMSTB > config ARCH_EXYNOS > bool "ARMv8 based Samsung Exynos SoC family" > select COMMON_CLK_SAMSUNG > + select CLKSRC_EXYNOS_MCT > select EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS > select EXYNOS_PMU > select HAVE_S3C2410_WATCHDOG if WATCHDOG > I tested it on Exynos5433-based TM2 board. Reviewed-by: Chanwoo Choi Tested-by: Chanwoo Choi -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH] Kbuild: Hide Clang's -Wempty-body behind W=1
Hi Masahiro,
On Wed, Oct 17, 2018 at 01:48:46PM +0900, Masahiro Yamada wrote:
> Hi Nathan,
>
>
> On Tue, Oct 16, 2018 at 11:15 AM Nathan Chancellor
> wrote:
> >
> > There are only a few instances of this warning in an arm64 allyesconfig
> > build but none of them appear useful. I believe the intention of the
> > warning is to avoid situations like this:
> >
> > if (condition);
> > statement;
> >
> > where the user really intended
> >
> > if (condition)
> > statement;
> >
> > However, these instances have already been caught by GCC's warning about
> > misleading indentation
>
>
> Right, the example above is caught by -Wmisleading-indentation.
>
> However, the following is not.
>
>if (condition)
> ;
>
>
>
> So, -Wempty-body is a kind of different thing,
> and still useful in my opinion.
>
>
>
> > so the remaining warnings are about loops that
> > fall into one of three categories:
> >
> > 1. Execute a function unconditionally (avoiding a useless variable to
> >hold the return value):
> >
> > drivers/isdn/hisax/hfc_pci.c:131:34: warning: if statement has empty body
> > [-Wempty-body]
> > if (Read_hfc(cs, HFCPCI_INT_S1));
>
>
> I think this is a real bug,
> then -Wempty-body finally caught it.
> (but -Wmisleading-indentation cannot catch it.)
>
>
>
> It is wrong to enclose a non-effective statement with 'if ();'
> just for suppressing another warning.
>
>
>
> Read_hfc(cs, HFCPCI_INT_S1);
>
> would emit this warning.
>
>
> In file included from drivers/isdn/hisax/hfc_pci.c:20:0:
> drivers/isdn/hisax/hfc_pci.c: In function ‘reset_hfcpci’:
> drivers/isdn/hisax/hfc_pci.h:232:25: warning: statement with no effect
> [-Wunused-value]
> #define Read_hfc(a, b) (*(((u_char *)a->hw.hfcpci.pci_io) + b))
> ~^~~
> drivers/isdn/hisax/hfc_pci.c:131:2: note: in expansion of macro ‘Read_hfc’
> Read_hfc(cs, HFCPCI_INT_S1);
> ^~~~
>
>
>
> The root cause is missing 'volatile'
> while Read_hfc() is supposed to read out a HW register.
>
>
>
> #define Read_hfc(a, b) (*(((volatile u_char *)a->hw.hfcpci.pci_io) + b))
>
> will be a correct fix.
> (or just use a standard accessor like readb(), ioread8(), etc.)
>
>
>
>
> if (Read_hfc(cs, HFCPCI_INT_S1));
>
> is optimized out by the compiler, so it is not working as expected.
>
>
>
> >
> > 2. Advancing a value to be used later on in the function like a pointer
> >or a count:
> >
> > drivers/atm/eni.c:244:48: warning: for loop has empty body
> > [-Wempty-body]
> > for (order = 0; (1 << order) < *size; order++);
> > ^
>
> As you noted in the commit log,
> Clang's -Wempty-body cares the location of a semi-colon,
> while GCC's one does not.
>
>
>
>
>
>for (order = 0; (1 << order) < *size; order++)
> ;
>
> is fine, and more readable in my opinion.
>
>
>
>
> > 3. Busy waiting:
> >
> > drivers/atm/zatm.c:513:7: warning: while loop has empty body
> > [-Wempty-body]
> > zwait;
> > ^
>
>
> Again, Clang is fine with an empty body in while() loop,
> but just picky about the semi-colon location.
>
> For this particular case, how about something like this?
>
>
> #define zwait do {} while (zin(CMR) & uPD98401_BUSY)
>
>
>
>
>
> I think an even better fix is
>
> #define zwait() do {} while (zin(CMR) & uPD98401_BUSY)
>
>
>
> then, fix-up all
>
>zwait;
>
> to
>
>zwait();
>
>
>
>
>
>
> > None of these uses are problematic or need to be addressed.
>
>
> The first pattern is really problematic, and need to be addressed.
>
> I want to keep -Wempty-body enabled
> to find out potential issues.
>
> Please let me know if you see other patterns difficult to fix.
>
>
>
Thank you very much for the quick feedback, this all sounds reasonable.
I will go ahead and dig into these further and send out patches to
address them.
Much appreciated,
Nathan
>
>
> > Clang
> > suggests moving the semi-colon to the next line to silence these
> > warnings but that defeats the purpose of the compact nature of these
> > constructs so just hide the warning behind W=1 so its use can still be
> > audited but it won't polute a regular build.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/42
> > Link: https://github.com/ClangBuiltLinux/linux/issues/66
> > Signed-off-by: Nathan Chancellor
> > ---
> > scripts/Makefile.extrawarn | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> > index cf6cd0ef6975..8709d9d6faf1 100644
> > --- a/scripts/Makefile.extrawarn
> > +++ b/scripts/Makefile.extrawarn
> > @@ -11,6 +11,7 @@
> > # are not supported by all versions of the compiler
> > #
> > ==
> >
> > +KBUILD_CFLAGS += $(call cc-disable-warning, empty-body)
> > KBUILD_CFLAGS += $(call
Re: [PATCH v2 5/6] arm64: dts: exynos: Move arch-timer node to right place
Hi Marek,
On 2018년 10월 15일 21:31, Marek Szyprowski wrote:
> Move ARM architected timer device-tree node to the beginning of 'soc'
> node, to group it together with other ARM CPU core devices (like PMU).
>
> Signed-off-by: Marek Szyprowski
> ---
> arch/arm64/boot/dts/exynos/exynos5433.dtsi | 23 +++---
> 1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> index 2131f12364cb..fa20eb3495b3 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> @@ -255,6 +255,18 @@
> interrupt-affinity = <>, <>, <>, <>;
> };
>
> + timer: timer {
> + compatible = "arm,armv8-timer";
> + interrupts = + (GIC_CPU_MASK_SIMPLE(8) |
> IRQ_TYPE_LEVEL_HIGH)>,
> + + (GIC_CPU_MASK_SIMPLE(8) |
> IRQ_TYPE_LEVEL_HIGH)>,
> + + (GIC_CPU_MASK_SIMPLE(8) |
> IRQ_TYPE_LEVEL_HIGH)>,
> + + (GIC_CPU_MASK_SIMPLE(8) |
> IRQ_TYPE_LEVEL_HIGH)>;
> + };
> +
> chipid@1000 {
> compatible = "samsung,exynos4210-chipid";
> reg = <0x1000 0x100>;
> @@ -1750,17 +1762,6 @@
> };
> };
>
> - timer: timer {
> - compatible = "arm,armv8-timer";
> - interrupts = - (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>,
> - - (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>,
> - - (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>,
> - - (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
> - };
> };
>
> #include "exynos5433-bus.dtsi"
>
I tested it on Exynos5433-based TM2 board. It is well working.
Reviewed-by: Chanwoo Choi
Tested-by: Chanwoo Choi
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Re: [PATCH v2 5/6] arm64: dts: exynos: Move arch-timer node to right place
Hi Marek,
On 2018년 10월 15일 21:31, Marek Szyprowski wrote:
> Move ARM architected timer device-tree node to the beginning of 'soc'
> node, to group it together with other ARM CPU core devices (like PMU).
>
> Signed-off-by: Marek Szyprowski
> ---
> arch/arm64/boot/dts/exynos/exynos5433.dtsi | 23 +++---
> 1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> index 2131f12364cb..fa20eb3495b3 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> @@ -255,6 +255,18 @@
> interrupt-affinity = <>, <>, <>, <>;
> };
>
> + timer: timer {
> + compatible = "arm,armv8-timer";
> + interrupts = + (GIC_CPU_MASK_SIMPLE(8) |
> IRQ_TYPE_LEVEL_HIGH)>,
> + + (GIC_CPU_MASK_SIMPLE(8) |
> IRQ_TYPE_LEVEL_HIGH)>,
> + + (GIC_CPU_MASK_SIMPLE(8) |
> IRQ_TYPE_LEVEL_HIGH)>,
> + + (GIC_CPU_MASK_SIMPLE(8) |
> IRQ_TYPE_LEVEL_HIGH)>;
> + };
> +
> chipid@1000 {
> compatible = "samsung,exynos4210-chipid";
> reg = <0x1000 0x100>;
> @@ -1750,17 +1762,6 @@
> };
> };
>
> - timer: timer {
> - compatible = "arm,armv8-timer";
> - interrupts = - (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>,
> - - (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>,
> - - (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>,
> - - (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
> - };
> };
>
> #include "exynos5433-bus.dtsi"
>
I tested it on Exynos5433-based TM2 board. It is well working.
Reviewed-by: Chanwoo Choi
Tested-by: Chanwoo Choi
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Re: [PATCH v2 3/6] clocksource: exynos_mct: Add arch_timer cooperation mode for ARM64
Hi Marek,
I tested it about kernel booting and CPU hotplug through sysfs path
on ARM64 Exynos5433-based TM2 board. It is well working.
Reviewed-by: Chanwoo Choi
Tested-by: Chanwoo Choi
But,
I have a minor comment for code clean-up.
On exynos4_mct_starting_cpu() function, the initialization of 'evt' instance
are not mandatory because 'evt' instance is not registered
by clockevents_config_and_register().
On 2018년 10월 15일 21:31, Marek Szyprowski wrote:
> To get ARM Architected Timers working on Samsung Exynos SoCs, one has to
> first configure and enable Exynos Multi-Core Timer, because they both
> share some common hardware blocks. This patch adds a mode of cooperation
> with arch_timer driver, so kernel can use CP15 based timer interface via
> arch_timer driver, which is mandatory on ARM64. In such mode driver only
> configures MCT registers and starts the timer but don't register any
> clocksource or events in the system. Those are left to be handled by
> arch_timer driver.
>
> Signed-off-by: Marek Szyprowski
> ---
> drivers/clocksource/exynos_mct.c | 52 +---
> 1 file changed, 35 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/clocksource/exynos_mct.c
> b/drivers/clocksource/exynos_mct.c
> index a379f11fad2d..06cd30a6d59a 100644
> --- a/drivers/clocksource/exynos_mct.c
> +++ b/drivers/clocksource/exynos_mct.c
> @@ -57,6 +57,7 @@
> #define TICK_BASE_CNT1
>
> enum {
> + MCT_INT_NONE = 0,
> MCT_INT_SPI,
> MCT_INT_PPI
> };
> @@ -238,6 +239,9 @@ static int __init exynos4_clocksource_init(void)
> {
> exynos4_mct_frc_start();
>
> + if (!mct_int_type)
> + return 0;
> +
> #if defined(CONFIG_ARM)
> exynos4_delay_timer.read_current_timer = _read_current_timer;
> exynos4_delay_timer.freq = clk_rate;
> @@ -343,6 +347,9 @@ static struct irqaction mct_comp_event_irq = {
>
> static int exynos4_clockevent_init(void)
> {
> + if (!mct_int_type)
> + return 0;
> +
> mct_comp_device.cpumask = cpumask_of(0);
> clockevents_config_and_register(_comp_device, clk_rate,
> 0xf, 0x);
> @@ -476,12 +483,12 @@ static int exynos4_mct_starting_cpu(unsigned int cpu)
>
> irq_force_affinity(evt->irq, cpumask_of(cpu));
> enable_irq(evt->irq);
> - } else {
> + } else if (mct_int_type == MCT_INT_PPI) {
> enable_percpu_irq(mct_irqs[MCT_L0_IRQ], 0);
> }
> - clockevents_config_and_register(evt, clk_rate / (TICK_BASE_CNT + 1),
> - 0xf, 0x7fff);
> -
> + if (mct_int_type)
> + clockevents_config_and_register(evt,
> +clk_rate / (TICK_BASE_CNT + 1), 0xf, 0x7fff);
> return 0;
> }
>
> @@ -496,7 +503,7 @@ static int exynos4_mct_dying_cpu(unsigned int cpu)
> if (evt->irq != -1)
> disable_irq_nosync(evt->irq);
> exynos4_mct_write(0x1, mevt->base + MCT_L_INT_CSTAT_OFFSET);
> - } else {
> + } else if (mct_int_type == MCT_INT_PPI) {
> disable_percpu_irq(mct_irqs[MCT_L0_IRQ]);
> }
> return 0;
> @@ -529,7 +536,7 @@ static int __init exynos4_timer_resources(struct
> device_node *np, void __iomem *
>_mct_tick);
> WARN(err, "MCT: can't request IRQ %d (%d)\n",
>mct_irqs[MCT_L0_IRQ], err);
> - } else {
> + } else if (mct_int_type == MCT_INT_SPI) {
> for_each_possible_cpu(cpu) {
> int mct_irq = mct_irqs[MCT_L0_IRQ + cpu];
> struct mct_clock_event_device *pcpu_mevt =
> @@ -564,7 +571,7 @@ static int __init exynos4_timer_resources(struct
> device_node *np, void __iomem *
> out_irq:
> if (mct_int_type == MCT_INT_PPI) {
> free_percpu_irq(mct_irqs[MCT_L0_IRQ], _mct_tick);
> - } else {
> + } else if (mct_int_type == MCT_INT_SPI) {
> for_each_possible_cpu(cpu) {
> struct mct_clock_event_device *pcpu_mevt =
> per_cpu_ptr(_mct_tick, cpu);
> @@ -585,17 +592,28 @@ static int __init mct_init_dt(struct device_node *np,
> unsigned int int_type)
>
> mct_int_type = int_type;
>
> - /* This driver uses only one global timer interrupt */
> - mct_irqs[MCT_G0_IRQ] = irq_of_parse_and_map(np, MCT_G0_IRQ);
> + if (IS_ENABLED(CONFIG_ARM64) && IS_ENABLED(CONFIG_ARM_ARCH_TIMER)) {
> + struct device_node *np = of_find_compatible_node(NULL, NULL,
> + "arm,armv8-timer");
> + if (np) {
> + mct_int_type = MCT_INT_NONE;
> + of_node_put(np);
> + }
> + }
>
> - /*
> - * Find out the number of local irqs specified. The local
> - * timer irqs are specified after the four
Re: [PATCH v2 3/6] clocksource: exynos_mct: Add arch_timer cooperation mode for ARM64
Hi Marek,
I tested it about kernel booting and CPU hotplug through sysfs path
on ARM64 Exynos5433-based TM2 board. It is well working.
Reviewed-by: Chanwoo Choi
Tested-by: Chanwoo Choi
But,
I have a minor comment for code clean-up.
On exynos4_mct_starting_cpu() function, the initialization of 'evt' instance
are not mandatory because 'evt' instance is not registered
by clockevents_config_and_register().
On 2018년 10월 15일 21:31, Marek Szyprowski wrote:
> To get ARM Architected Timers working on Samsung Exynos SoCs, one has to
> first configure and enable Exynos Multi-Core Timer, because they both
> share some common hardware blocks. This patch adds a mode of cooperation
> with arch_timer driver, so kernel can use CP15 based timer interface via
> arch_timer driver, which is mandatory on ARM64. In such mode driver only
> configures MCT registers and starts the timer but don't register any
> clocksource or events in the system. Those are left to be handled by
> arch_timer driver.
>
> Signed-off-by: Marek Szyprowski
> ---
> drivers/clocksource/exynos_mct.c | 52 +---
> 1 file changed, 35 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/clocksource/exynos_mct.c
> b/drivers/clocksource/exynos_mct.c
> index a379f11fad2d..06cd30a6d59a 100644
> --- a/drivers/clocksource/exynos_mct.c
> +++ b/drivers/clocksource/exynos_mct.c
> @@ -57,6 +57,7 @@
> #define TICK_BASE_CNT1
>
> enum {
> + MCT_INT_NONE = 0,
> MCT_INT_SPI,
> MCT_INT_PPI
> };
> @@ -238,6 +239,9 @@ static int __init exynos4_clocksource_init(void)
> {
> exynos4_mct_frc_start();
>
> + if (!mct_int_type)
> + return 0;
> +
> #if defined(CONFIG_ARM)
> exynos4_delay_timer.read_current_timer = _read_current_timer;
> exynos4_delay_timer.freq = clk_rate;
> @@ -343,6 +347,9 @@ static struct irqaction mct_comp_event_irq = {
>
> static int exynos4_clockevent_init(void)
> {
> + if (!mct_int_type)
> + return 0;
> +
> mct_comp_device.cpumask = cpumask_of(0);
> clockevents_config_and_register(_comp_device, clk_rate,
> 0xf, 0x);
> @@ -476,12 +483,12 @@ static int exynos4_mct_starting_cpu(unsigned int cpu)
>
> irq_force_affinity(evt->irq, cpumask_of(cpu));
> enable_irq(evt->irq);
> - } else {
> + } else if (mct_int_type == MCT_INT_PPI) {
> enable_percpu_irq(mct_irqs[MCT_L0_IRQ], 0);
> }
> - clockevents_config_and_register(evt, clk_rate / (TICK_BASE_CNT + 1),
> - 0xf, 0x7fff);
> -
> + if (mct_int_type)
> + clockevents_config_and_register(evt,
> +clk_rate / (TICK_BASE_CNT + 1), 0xf, 0x7fff);
> return 0;
> }
>
> @@ -496,7 +503,7 @@ static int exynos4_mct_dying_cpu(unsigned int cpu)
> if (evt->irq != -1)
> disable_irq_nosync(evt->irq);
> exynos4_mct_write(0x1, mevt->base + MCT_L_INT_CSTAT_OFFSET);
> - } else {
> + } else if (mct_int_type == MCT_INT_PPI) {
> disable_percpu_irq(mct_irqs[MCT_L0_IRQ]);
> }
> return 0;
> @@ -529,7 +536,7 @@ static int __init exynos4_timer_resources(struct
> device_node *np, void __iomem *
>_mct_tick);
> WARN(err, "MCT: can't request IRQ %d (%d)\n",
>mct_irqs[MCT_L0_IRQ], err);
> - } else {
> + } else if (mct_int_type == MCT_INT_SPI) {
> for_each_possible_cpu(cpu) {
> int mct_irq = mct_irqs[MCT_L0_IRQ + cpu];
> struct mct_clock_event_device *pcpu_mevt =
> @@ -564,7 +571,7 @@ static int __init exynos4_timer_resources(struct
> device_node *np, void __iomem *
> out_irq:
> if (mct_int_type == MCT_INT_PPI) {
> free_percpu_irq(mct_irqs[MCT_L0_IRQ], _mct_tick);
> - } else {
> + } else if (mct_int_type == MCT_INT_SPI) {
> for_each_possible_cpu(cpu) {
> struct mct_clock_event_device *pcpu_mevt =
> per_cpu_ptr(_mct_tick, cpu);
> @@ -585,17 +592,28 @@ static int __init mct_init_dt(struct device_node *np,
> unsigned int int_type)
>
> mct_int_type = int_type;
>
> - /* This driver uses only one global timer interrupt */
> - mct_irqs[MCT_G0_IRQ] = irq_of_parse_and_map(np, MCT_G0_IRQ);
> + if (IS_ENABLED(CONFIG_ARM64) && IS_ENABLED(CONFIG_ARM_ARCH_TIMER)) {
> + struct device_node *np = of_find_compatible_node(NULL, NULL,
> + "arm,armv8-timer");
> + if (np) {
> + mct_int_type = MCT_INT_NONE;
> + of_node_put(np);
> + }
> + }
>
> - /*
> - * Find out the number of local irqs specified. The local
> - * timer irqs are specified after the four
Re: [PATCH] Kbuild: Hide Clang's -Wempty-body behind W=1
Hi Nathan,
On Tue, Oct 16, 2018 at 11:15 AM Nathan Chancellor
wrote:
>
> There are only a few instances of this warning in an arm64 allyesconfig
> build but none of them appear useful. I believe the intention of the
> warning is to avoid situations like this:
>
> if (condition);
> statement;
>
> where the user really intended
>
> if (condition)
> statement;
>
> However, these instances have already been caught by GCC's warning about
> misleading indentation
Right, the example above is caught by -Wmisleading-indentation.
However, the following is not.
if (condition)
;
So, -Wempty-body is a kind of different thing,
and still useful in my opinion.
> so the remaining warnings are about loops that
> fall into one of three categories:
>
> 1. Execute a function unconditionally (avoiding a useless variable to
>hold the return value):
>
> drivers/isdn/hisax/hfc_pci.c:131:34: warning: if statement has empty body
> [-Wempty-body]
> if (Read_hfc(cs, HFCPCI_INT_S1));
I think this is a real bug,
then -Wempty-body finally caught it.
(but -Wmisleading-indentation cannot catch it.)
It is wrong to enclose a non-effective statement with 'if ();'
just for suppressing another warning.
Read_hfc(cs, HFCPCI_INT_S1);
would emit this warning.
In file included from drivers/isdn/hisax/hfc_pci.c:20:0:
drivers/isdn/hisax/hfc_pci.c: In function ‘reset_hfcpci’:
drivers/isdn/hisax/hfc_pci.h:232:25: warning: statement with no effect
[-Wunused-value]
#define Read_hfc(a, b) (*(((u_char *)a->hw.hfcpci.pci_io) + b))
~^~~
drivers/isdn/hisax/hfc_pci.c:131:2: note: in expansion of macro ‘Read_hfc’
Read_hfc(cs, HFCPCI_INT_S1);
^~~~
The root cause is missing 'volatile'
while Read_hfc() is supposed to read out a HW register.
#define Read_hfc(a, b) (*(((volatile u_char *)a->hw.hfcpci.pci_io) + b))
will be a correct fix.
(or just use a standard accessor like readb(), ioread8(), etc.)
if (Read_hfc(cs, HFCPCI_INT_S1));
is optimized out by the compiler, so it is not working as expected.
>
> 2. Advancing a value to be used later on in the function like a pointer
>or a count:
>
> drivers/atm/eni.c:244:48: warning: for loop has empty body
> [-Wempty-body]
> for (order = 0; (1 << order) < *size; order++);
> ^
As you noted in the commit log,
Clang's -Wempty-body cares the location of a semi-colon,
while GCC's one does not.
for (order = 0; (1 << order) < *size; order++)
;
is fine, and more readable in my opinion.
> 3. Busy waiting:
>
> drivers/atm/zatm.c:513:7: warning: while loop has empty body
> [-Wempty-body]
> zwait;
> ^
Again, Clang is fine with an empty body in while() loop,
but just picky about the semi-colon location.
For this particular case, how about something like this?
#define zwait do {} while (zin(CMR) & uPD98401_BUSY)
I think an even better fix is
#define zwait() do {} while (zin(CMR) & uPD98401_BUSY)
then, fix-up all
zwait;
to
zwait();
> None of these uses are problematic or need to be addressed.
The first pattern is really problematic, and need to be addressed.
I want to keep -Wempty-body enabled
to find out potential issues.
Please let me know if you see other patterns difficult to fix.
> Clang
> suggests moving the semi-colon to the next line to silence these
> warnings but that defeats the purpose of the compact nature of these
> constructs so just hide the warning behind W=1 so its use can still be
> audited but it won't polute a regular build.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/42
> Link: https://github.com/ClangBuiltLinux/linux/issues/66
> Signed-off-by: Nathan Chancellor
> ---
> scripts/Makefile.extrawarn | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> index cf6cd0ef6975..8709d9d6faf1 100644
> --- a/scripts/Makefile.extrawarn
> +++ b/scripts/Makefile.extrawarn
> @@ -11,6 +11,7 @@
> # are not supported by all versions of the compiler
> # ==
>
> +KBUILD_CFLAGS += $(call cc-disable-warning, empty-body)
> KBUILD_CFLAGS += $(call cc-disable-warning, packed-not-aligned)
>
> ifeq ("$(origin W)", "command line")
> @@ -32,6 +33,7 @@ warning-1 += $(call cc-option, -Wpacked-not-aligned)
> warning-1 += $(call cc-option, -Wstringop-truncation)
> warning-1 += $(call cc-disable-warning, missing-field-initializers)
> warning-1 += $(call cc-disable-warning, sign-compare)
> +warning-1 += $(call cc-option, -Wempty-body)
>
> warning-2 := -Waggregate-return
> warning-2 += -Wcast-align
> --
> 2.19.1
>
--
Best Regards
Masahiro Yamada
Re: [PATCH] Kbuild: Hide Clang's -Wempty-body behind W=1
Hi Nathan,
On Tue, Oct 16, 2018 at 11:15 AM Nathan Chancellor
wrote:
>
> There are only a few instances of this warning in an arm64 allyesconfig
> build but none of them appear useful. I believe the intention of the
> warning is to avoid situations like this:
>
> if (condition);
> statement;
>
> where the user really intended
>
> if (condition)
> statement;
>
> However, these instances have already been caught by GCC's warning about
> misleading indentation
Right, the example above is caught by -Wmisleading-indentation.
However, the following is not.
if (condition)
;
So, -Wempty-body is a kind of different thing,
and still useful in my opinion.
> so the remaining warnings are about loops that
> fall into one of three categories:
>
> 1. Execute a function unconditionally (avoiding a useless variable to
>hold the return value):
>
> drivers/isdn/hisax/hfc_pci.c:131:34: warning: if statement has empty body
> [-Wempty-body]
> if (Read_hfc(cs, HFCPCI_INT_S1));
I think this is a real bug,
then -Wempty-body finally caught it.
(but -Wmisleading-indentation cannot catch it.)
It is wrong to enclose a non-effective statement with 'if ();'
just for suppressing another warning.
Read_hfc(cs, HFCPCI_INT_S1);
would emit this warning.
In file included from drivers/isdn/hisax/hfc_pci.c:20:0:
drivers/isdn/hisax/hfc_pci.c: In function ‘reset_hfcpci’:
drivers/isdn/hisax/hfc_pci.h:232:25: warning: statement with no effect
[-Wunused-value]
#define Read_hfc(a, b) (*(((u_char *)a->hw.hfcpci.pci_io) + b))
~^~~
drivers/isdn/hisax/hfc_pci.c:131:2: note: in expansion of macro ‘Read_hfc’
Read_hfc(cs, HFCPCI_INT_S1);
^~~~
The root cause is missing 'volatile'
while Read_hfc() is supposed to read out a HW register.
#define Read_hfc(a, b) (*(((volatile u_char *)a->hw.hfcpci.pci_io) + b))
will be a correct fix.
(or just use a standard accessor like readb(), ioread8(), etc.)
if (Read_hfc(cs, HFCPCI_INT_S1));
is optimized out by the compiler, so it is not working as expected.
>
> 2. Advancing a value to be used later on in the function like a pointer
>or a count:
>
> drivers/atm/eni.c:244:48: warning: for loop has empty body
> [-Wempty-body]
> for (order = 0; (1 << order) < *size; order++);
> ^
As you noted in the commit log,
Clang's -Wempty-body cares the location of a semi-colon,
while GCC's one does not.
for (order = 0; (1 << order) < *size; order++)
;
is fine, and more readable in my opinion.
> 3. Busy waiting:
>
> drivers/atm/zatm.c:513:7: warning: while loop has empty body
> [-Wempty-body]
> zwait;
> ^
Again, Clang is fine with an empty body in while() loop,
but just picky about the semi-colon location.
For this particular case, how about something like this?
#define zwait do {} while (zin(CMR) & uPD98401_BUSY)
I think an even better fix is
#define zwait() do {} while (zin(CMR) & uPD98401_BUSY)
then, fix-up all
zwait;
to
zwait();
> None of these uses are problematic or need to be addressed.
The first pattern is really problematic, and need to be addressed.
I want to keep -Wempty-body enabled
to find out potential issues.
Please let me know if you see other patterns difficult to fix.
> Clang
> suggests moving the semi-colon to the next line to silence these
> warnings but that defeats the purpose of the compact nature of these
> constructs so just hide the warning behind W=1 so its use can still be
> audited but it won't polute a regular build.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/42
> Link: https://github.com/ClangBuiltLinux/linux/issues/66
> Signed-off-by: Nathan Chancellor
> ---
> scripts/Makefile.extrawarn | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> index cf6cd0ef6975..8709d9d6faf1 100644
> --- a/scripts/Makefile.extrawarn
> +++ b/scripts/Makefile.extrawarn
> @@ -11,6 +11,7 @@
> # are not supported by all versions of the compiler
> # ==
>
> +KBUILD_CFLAGS += $(call cc-disable-warning, empty-body)
> KBUILD_CFLAGS += $(call cc-disable-warning, packed-not-aligned)
>
> ifeq ("$(origin W)", "command line")
> @@ -32,6 +33,7 @@ warning-1 += $(call cc-option, -Wpacked-not-aligned)
> warning-1 += $(call cc-option, -Wstringop-truncation)
> warning-1 += $(call cc-disable-warning, missing-field-initializers)
> warning-1 += $(call cc-disable-warning, sign-compare)
> +warning-1 += $(call cc-option, -Wempty-body)
>
> warning-2 := -Waggregate-return
> warning-2 += -Wcast-align
> --
> 2.19.1
>
--
Best Regards
Masahiro Yamada
Re: [RFC][PATCHv2 1/4] panic: avoid deadlocks in re-entrant console drivers
On (10/16/18 14:04), Sergey Senozhatsky wrote: > > Fix this by setting oops_in_progress before console_flush_on_panic(), > so re-entrant console drivers will trylock the port->lock instead of > spinning on it forever. > Just a small note: Regardless of what's going to happen to the series, this patch deserves attention. -ss
Re: [RFC][PATCHv2 1/4] panic: avoid deadlocks in re-entrant console drivers
On (10/16/18 14:04), Sergey Senozhatsky wrote: > > Fix this by setting oops_in_progress before console_flush_on_panic(), > so re-entrant console drivers will trylock the port->lock instead of > spinning on it forever. > Just a small note: Regardless of what's going to happen to the series, this patch deserves attention. -ss
Re: [Lkcamp] [PATCH 0/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS
Hi Leonardo, Thanks for the patches. On 10/16/18 9:07 PM, Leonardo Brás wrote: > This patchset add -Wshadow=local on KBUILD_HOSTCFLAGS and fixes > all code that show this warning. > > The third patch was already submitted, but was not merged yet. > I like to think it's part of this patchset, but if it was > already merged, please ignore it. You can check if it was merged in the maintainers' tree, if it is not there then it is not merged :) imho, I would prefer if you submitted it as a v2 with an explanation of what changed since v1 (e.g. new commits) or even stating that nothing has changed and you just regrouped it. Or better, you could also not re-submit it and just point out that this series depends on a previous commit (with a link to it). > > Leonardo Brás (4): > Adds -Wshadow=local on KBUILD_HOSTCFLAGS > Renames variable to fix shadow warning. > kbuild: Removes unnecessary shadowed local variable and optimize > testing. > Changes macro usage to avoid shadowing a variable. > > Makefile | 2 +- > arch/x86/entry/vdso/vdso2c.h | 4 ++-- > scripts/asn1_compiler.c | 7 +++ > scripts/mod/file2alias.c | 14 -- > 4 files changed, 14 insertions(+), 13 deletions(-) > I hope this helps. Regards, Helen
Re: [Lkcamp] [PATCH 0/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS
Hi Leonardo, Thanks for the patches. On 10/16/18 9:07 PM, Leonardo Brás wrote: > This patchset add -Wshadow=local on KBUILD_HOSTCFLAGS and fixes > all code that show this warning. > > The third patch was already submitted, but was not merged yet. > I like to think it's part of this patchset, but if it was > already merged, please ignore it. You can check if it was merged in the maintainers' tree, if it is not there then it is not merged :) imho, I would prefer if you submitted it as a v2 with an explanation of what changed since v1 (e.g. new commits) or even stating that nothing has changed and you just regrouped it. Or better, you could also not re-submit it and just point out that this series depends on a previous commit (with a link to it). > > Leonardo Brás (4): > Adds -Wshadow=local on KBUILD_HOSTCFLAGS > Renames variable to fix shadow warning. > kbuild: Removes unnecessary shadowed local variable and optimize > testing. > Changes macro usage to avoid shadowing a variable. > > Makefile | 2 +- > arch/x86/entry/vdso/vdso2c.h | 4 ++-- > scripts/asn1_compiler.c | 7 +++ > scripts/mod/file2alias.c | 14 -- > 4 files changed, 14 insertions(+), 13 deletions(-) > I hope this helps. Regards, Helen
Re: [RFC][PATCHv2 2/4] printk: move printk_safe macros to printk header
On (10/16/18 14:54), Peter Zijlstra wrote: > On Tue, Oct 16, 2018 at 09:27:34PM +0900, Sergey Senozhatsky wrote: > > per-CPU printk_safe _semi-magic_ makes some things simple to handle. > > We can't just remove per-CPU buffers and add a wake_up_process() at > > the bottom of vprintk_emit(). Because this will deadlock: > > > > printk() > >wake_up_process() > > try_to_wake_up() > > raw_spin_lock_irqsave() > > > > printk() > >wake_up_process() > > try_to_wake_up() > > raw_spin_lock_irqsave() > > > > So we still need some amount of per-CPU printk() semi-magic anyway. > > All we need is 4 max-line-length buffers per-CPU. Nothing more. OK, similar to what Steven did with cpu_buffer->current_context. > The above trainwreck is the direct result of forcing synchronous > printk'ing (which I'm otherwise a big fan of, but regular console > drivers stink and are unfixable). Yep. > > And printk-kthread offloding will not eliminate the need of > > printk_deferred(). > > Why not? printk() will reduce to a lockless buffer insert. IOW _all_ > printk is deferred. Aha! Interesting. I didn't realize you were talking about "all printk()-s are deferred". OK, jump to the last part of this mail. > All you need are 4 max-line-length buffers per CPU and a global/shared > lockless buffer. > > printk will determine the current context: > > task, softirq, hardirq or NMI > > and pick the corresponding per-cpu line buffer and do the vsnprintf() > thing. Then we have the actual line length and content. With the length > we reserve the bytes from the global buffer, we memcpy into the buffer > and commit. > > Done. > > The printk thread will observe it lags behind the buffer head and will > start printk-ing crud from task context. [you can skip this part] This probably will be a bit more hairy. logbuf is written to by many sources and is read from by many sides, including user-space [both read() and write()]. So we will need more flags/magic around memcpy(). A simple, "grab the logbuf entry, set the proper offset to point to the next available logbuf record and then do memcpy()" won't suffice. We need a flag for "memcpy() complete, we can read this entry". Otherwise: CPU0CPU1CPU2CPU3 printk printk printk_kthread logbuf_entry A logbuf_entry B syslog(read all)call_console_drivers memcpy memcpy read unfinished print unfinished A and B A and B [..] > > We do, however, have loads of problems with all those dependencies which > > come from serial drivers and friends: timekeeping, scheduler (scheduler > > is brilliant and cool, but we do have some deadlocks in printk because of > > it ;), tty, net, MM, wq, etc. So I generally like the idea of "detached > > serial consoles" (that's what I call it). IOW, elimination of the direct > > printk -> serial console path. > > Right; we need to get rid of that in the generic case. Only allow > lockless consoles (earlycon) to be used synchonously. With maybe a > special case for the BUG/PANIC case to push things out ASAP. [..] > > So, unless I'm missing something, things are not entirely that simple: > > - throw away printk_safe semi-magic > > - add a lockless logbuf > > - add wake_up_process() to vprintk_emit(). > > No, no wakups. irq_work to wake the printk-thread, at most. All right. OK. So we are on the same page here: printk has internal locks - logbuf spin_lock; and external locks - all the scheduler locks, console_sem, net, tty, wq, you name it. printk() is not aware of those external locks; the only way to fix it is to remove them from printk(). And that's why "turn printk() into printk_deferred() and fix printk() deadlocks in general case" was my final proposal at the 2016 KS, NM, USA [1] (grep for printk_deferred). I mentioned this idea several times since then, and even sent a patch, doing this "printk is now printk_deferred unless we are in panic" thing. As far as I remember, back then the idea/patch were rejected [2], and one of reviewers even hinted that I was crazy :-) I have absolutely no issues with that, but, considering past experiences, I'd really like to: - Have more opinions on this. People please speak out. - Have clear "let's do it" from Cc-ed people. If we are really doing this, then let's split it and have incremental changes. Namely, what I suggest is: - keep internal printk lock - logbuf lock for now; we know how to handle it. I promise. - keep printk_safe for now, we need it to deal with logbuf lock - keep printk_safe completely internal to printk - add printk_kthread - do printk()->irq_work()->wake_up_process(printk_kthread) change and remove external locks dependency - use direct printk() for panic() case - do something about early_printk That's big enough already. >From there, once we land this
Re: [RFC][PATCHv2 2/4] printk: move printk_safe macros to printk header
On (10/16/18 14:54), Peter Zijlstra wrote: > On Tue, Oct 16, 2018 at 09:27:34PM +0900, Sergey Senozhatsky wrote: > > per-CPU printk_safe _semi-magic_ makes some things simple to handle. > > We can't just remove per-CPU buffers and add a wake_up_process() at > > the bottom of vprintk_emit(). Because this will deadlock: > > > > printk() > >wake_up_process() > > try_to_wake_up() > > raw_spin_lock_irqsave() > > > > printk() > >wake_up_process() > > try_to_wake_up() > > raw_spin_lock_irqsave() > > > > So we still need some amount of per-CPU printk() semi-magic anyway. > > All we need is 4 max-line-length buffers per-CPU. Nothing more. OK, similar to what Steven did with cpu_buffer->current_context. > The above trainwreck is the direct result of forcing synchronous > printk'ing (which I'm otherwise a big fan of, but regular console > drivers stink and are unfixable). Yep. > > And printk-kthread offloding will not eliminate the need of > > printk_deferred(). > > Why not? printk() will reduce to a lockless buffer insert. IOW _all_ > printk is deferred. Aha! Interesting. I didn't realize you were talking about "all printk()-s are deferred". OK, jump to the last part of this mail. > All you need are 4 max-line-length buffers per CPU and a global/shared > lockless buffer. > > printk will determine the current context: > > task, softirq, hardirq or NMI > > and pick the corresponding per-cpu line buffer and do the vsnprintf() > thing. Then we have the actual line length and content. With the length > we reserve the bytes from the global buffer, we memcpy into the buffer > and commit. > > Done. > > The printk thread will observe it lags behind the buffer head and will > start printk-ing crud from task context. [you can skip this part] This probably will be a bit more hairy. logbuf is written to by many sources and is read from by many sides, including user-space [both read() and write()]. So we will need more flags/magic around memcpy(). A simple, "grab the logbuf entry, set the proper offset to point to the next available logbuf record and then do memcpy()" won't suffice. We need a flag for "memcpy() complete, we can read this entry". Otherwise: CPU0CPU1CPU2CPU3 printk printk printk_kthread logbuf_entry A logbuf_entry B syslog(read all)call_console_drivers memcpy memcpy read unfinished print unfinished A and B A and B [..] > > We do, however, have loads of problems with all those dependencies which > > come from serial drivers and friends: timekeeping, scheduler (scheduler > > is brilliant and cool, but we do have some deadlocks in printk because of > > it ;), tty, net, MM, wq, etc. So I generally like the idea of "detached > > serial consoles" (that's what I call it). IOW, elimination of the direct > > printk -> serial console path. > > Right; we need to get rid of that in the generic case. Only allow > lockless consoles (earlycon) to be used synchonously. With maybe a > special case for the BUG/PANIC case to push things out ASAP. [..] > > So, unless I'm missing something, things are not entirely that simple: > > - throw away printk_safe semi-magic > > - add a lockless logbuf > > - add wake_up_process() to vprintk_emit(). > > No, no wakups. irq_work to wake the printk-thread, at most. All right. OK. So we are on the same page here: printk has internal locks - logbuf spin_lock; and external locks - all the scheduler locks, console_sem, net, tty, wq, you name it. printk() is not aware of those external locks; the only way to fix it is to remove them from printk(). And that's why "turn printk() into printk_deferred() and fix printk() deadlocks in general case" was my final proposal at the 2016 KS, NM, USA [1] (grep for printk_deferred). I mentioned this idea several times since then, and even sent a patch, doing this "printk is now printk_deferred unless we are in panic" thing. As far as I remember, back then the idea/patch were rejected [2], and one of reviewers even hinted that I was crazy :-) I have absolutely no issues with that, but, considering past experiences, I'd really like to: - Have more opinions on this. People please speak out. - Have clear "let's do it" from Cc-ed people. If we are really doing this, then let's split it and have incremental changes. Namely, what I suggest is: - keep internal printk lock - logbuf lock for now; we know how to handle it. I promise. - keep printk_safe for now, we need it to deal with logbuf lock - keep printk_safe completely internal to printk - add printk_kthread - do printk()->irq_work()->wake_up_process(printk_kthread) change and remove external locks dependency - use direct printk() for panic() case - do something about early_printk That's big enough already. >From there, once we land this
Re: [Lkcamp] [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS
Hi Leonardo, Thanks for the patch. On 10/16/18 9:08 PM, Leonardo Brás wrote: > Adds -Wshadow=local on KBUILD_HOSTCFLAGS to show shadow warnings > on tools built for HOST. > > Signed-off-by: Leonardo Brás > --- > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index e8b599b4dcde..fb0a9ac195e7 100644 > --- a/Makefile > +++ b/Makefile > @@ -360,7 +360,7 @@ HOST_LFS_LIBS := $(shell getconf LFS_LIBS 2>/dev/null) > > HOSTCC = gcc > HOSTCXX = g++ > -KBUILD_HOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \ > +KBUILD_HOSTCFLAGS := -Wall -Wshadow=local -Wmissing-prototypes > -Wstrict-prototypes -O2 \ > -fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS) \ > $(HOSTCFLAGS) > KBUILD_HOSTCXXFLAGS := -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS) > I believe this should be the last patch on this series and not the first one to avoid commits in between where we have those warnings. Regards Helen
Re: [Lkcamp] [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS
Hi Leonardo, Thanks for the patch. On 10/16/18 9:08 PM, Leonardo Brás wrote: > Adds -Wshadow=local on KBUILD_HOSTCFLAGS to show shadow warnings > on tools built for HOST. > > Signed-off-by: Leonardo Brás > --- > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index e8b599b4dcde..fb0a9ac195e7 100644 > --- a/Makefile > +++ b/Makefile > @@ -360,7 +360,7 @@ HOST_LFS_LIBS := $(shell getconf LFS_LIBS 2>/dev/null) > > HOSTCC = gcc > HOSTCXX = g++ > -KBUILD_HOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \ > +KBUILD_HOSTCFLAGS := -Wall -Wshadow=local -Wmissing-prototypes > -Wstrict-prototypes -O2 \ > -fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS) \ > $(HOSTCFLAGS) > KBUILD_HOSTCXXFLAGS := -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS) > I believe this should be the last patch on this series and not the first one to avoid commits in between where we have those warnings. Regards Helen
Re: [Lkcamp] [PATCH 4/4] Changes macro usage to avoid shadowing a variable.
Hi Leonardo,
Thanks for the patch, just some small comments below.
On 10/16/18 9:09 PM, Leonardo Brás wrote:
> Changes the usage of DEF_FIELD_ADDR in this function to create a
> reference and operate over it using an aux variable.
> It also changes the loop logic used to find duplicates, to avoid
> creating another variable.
>
> Signed-off-by: Leonardo Brás
> ---
> scripts/mod/file2alias.c | 14 --
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index 7be43697ff84..9ea1db2aefdb 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -641,25 +641,27 @@ static void do_pnp_card_entries(void *symval, unsigned
> long size,
> unsigned int i;
>
> device_id_check(mod->name, "pnp", size, id_size, symval);
> + DEF_FIELD_ADDR(symval, pnp_card_device_id, devs);
> + typeof(devs) devs_last;
>
> for (i = 0; i < count; i++) {
> unsigned int j;
> - DEF_FIELD_ADDR(symval + i*id_size, pnp_card_device_id, devs);
> + devs_last = devs + i * id_size;
>
> for (j = 0; j < PNP_MAX_DEVICES; j++) {
> - const char *id = (char *)(*devs)[j].id;
> - int i2, j2;
> + const char *id = (char *)(*devs_last)[j].id;
> + int j2;
> int dup = 0;
>
> if (!id[0])
> break;
>
> /* find duplicate, already added value */
> - for (i2 = 0; i2 < i && !dup; i2++) {
> - DEF_FIELD_ADDR(symval + i2*id_size,
> pnp_card_device_id, devs);
> + while ((devs_last -= id_size) >= devs) {
You forgot to consider the dup variable.
Also you inverted the order of this loop, I am not sure this is
important (I just took a quick look) but you need to be careful.
This is also hard to read, I would define another variant macro where
you can set any name of the variable, e.g.
#define DEF_FIELD_ADDR_VAR(m, devid, f, var) \
typeof(((struct devid *)0)->f) *var = ((m) + OFF_##devid##_##f)
In this way you don't need to change the logic, just the name of the
variable.
>
> for (j2 = 0; j2 < PNP_MAX_DEVICES; j2++) {
> - const char *id2 = (char
> *)(*devs)[j2].id;
> + const char *id2 =
> + (char *)(*devs_last)[j2].id;
>
> if (!id2[0])
> break;
>
Regards,
Helen
Re: [Lkcamp] [PATCH 4/4] Changes macro usage to avoid shadowing a variable.
Hi Leonardo,
Thanks for the patch, just some small comments below.
On 10/16/18 9:09 PM, Leonardo Brás wrote:
> Changes the usage of DEF_FIELD_ADDR in this function to create a
> reference and operate over it using an aux variable.
> It also changes the loop logic used to find duplicates, to avoid
> creating another variable.
>
> Signed-off-by: Leonardo Brás
> ---
> scripts/mod/file2alias.c | 14 --
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index 7be43697ff84..9ea1db2aefdb 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -641,25 +641,27 @@ static void do_pnp_card_entries(void *symval, unsigned
> long size,
> unsigned int i;
>
> device_id_check(mod->name, "pnp", size, id_size, symval);
> + DEF_FIELD_ADDR(symval, pnp_card_device_id, devs);
> + typeof(devs) devs_last;
>
> for (i = 0; i < count; i++) {
> unsigned int j;
> - DEF_FIELD_ADDR(symval + i*id_size, pnp_card_device_id, devs);
> + devs_last = devs + i * id_size;
>
> for (j = 0; j < PNP_MAX_DEVICES; j++) {
> - const char *id = (char *)(*devs)[j].id;
> - int i2, j2;
> + const char *id = (char *)(*devs_last)[j].id;
> + int j2;
> int dup = 0;
>
> if (!id[0])
> break;
>
> /* find duplicate, already added value */
> - for (i2 = 0; i2 < i && !dup; i2++) {
> - DEF_FIELD_ADDR(symval + i2*id_size,
> pnp_card_device_id, devs);
> + while ((devs_last -= id_size) >= devs) {
You forgot to consider the dup variable.
Also you inverted the order of this loop, I am not sure this is
important (I just took a quick look) but you need to be careful.
This is also hard to read, I would define another variant macro where
you can set any name of the variable, e.g.
#define DEF_FIELD_ADDR_VAR(m, devid, f, var) \
typeof(((struct devid *)0)->f) *var = ((m) + OFF_##devid##_##f)
In this way you don't need to change the logic, just the name of the
variable.
>
> for (j2 = 0; j2 < PNP_MAX_DEVICES; j2++) {
> - const char *id2 = (char
> *)(*devs)[j2].id;
> + const char *id2 =
> + (char *)(*devs_last)[j2].id;
>
> if (!id2[0])
> break;
>
Regards,
Helen
[PATCH] perf: Add Sparc jitdump support.
Signed-off-by: David S. Miller --- tools/perf/arch/sparc/Makefile | 2 ++ tools/perf/util/genelf.h | 6 ++ 2 files changed, 8 insertions(+) diff --git a/tools/perf/arch/sparc/Makefile b/tools/perf/arch/sparc/Makefile index 7fbca175099e..275dea7ff59a 100644 --- a/tools/perf/arch/sparc/Makefile +++ b/tools/perf/arch/sparc/Makefile @@ -1,3 +1,5 @@ ifndef NO_DWARF PERF_HAVE_DWARF_REGS := 1 endif + +PERF_HAVE_JITDUMP := 1 diff --git a/tools/perf/util/genelf.h b/tools/perf/util/genelf.h index de322d51c7fe..b72440bf9a79 100644 --- a/tools/perf/util/genelf.h +++ b/tools/perf/util/genelf.h @@ -29,6 +29,12 @@ int jit_add_debug_info(Elf *e, uint64_t code_addr, void *debug, int nr_debug_ent #elif defined(__powerpc__) #define GEN_ELF_ARCH EM_PPC #define GEN_ELF_CLASS ELFCLASS32 +#elif defined(__sparc__) && defined(__arch64__) +#define GEN_ELF_ARCH EM_SPARCV9 +#define GEN_ELF_CLASS ELFCLASS64 +#elif defined(__sparc__) +#define GEN_ELF_ARCH EM_SPARC +#define GEN_ELF_CLASS ELFCLASS32 #else #error "unsupported architecture" #endif -- 2.19.1.328.g5a0cc8aca
[PATCH] perf: Add Sparc jitdump support.
Signed-off-by: David S. Miller --- tools/perf/arch/sparc/Makefile | 2 ++ tools/perf/util/genelf.h | 6 ++ 2 files changed, 8 insertions(+) diff --git a/tools/perf/arch/sparc/Makefile b/tools/perf/arch/sparc/Makefile index 7fbca175099e..275dea7ff59a 100644 --- a/tools/perf/arch/sparc/Makefile +++ b/tools/perf/arch/sparc/Makefile @@ -1,3 +1,5 @@ ifndef NO_DWARF PERF_HAVE_DWARF_REGS := 1 endif + +PERF_HAVE_JITDUMP := 1 diff --git a/tools/perf/util/genelf.h b/tools/perf/util/genelf.h index de322d51c7fe..b72440bf9a79 100644 --- a/tools/perf/util/genelf.h +++ b/tools/perf/util/genelf.h @@ -29,6 +29,12 @@ int jit_add_debug_info(Elf *e, uint64_t code_addr, void *debug, int nr_debug_ent #elif defined(__powerpc__) #define GEN_ELF_ARCH EM_PPC #define GEN_ELF_CLASS ELFCLASS32 +#elif defined(__sparc__) && defined(__arch64__) +#define GEN_ELF_ARCH EM_SPARCV9 +#define GEN_ELF_CLASS ELFCLASS64 +#elif defined(__sparc__) +#define GEN_ELF_ARCH EM_SPARC +#define GEN_ELF_CLASS ELFCLASS32 #else #error "unsupported architecture" #endif -- 2.19.1.328.g5a0cc8aca
[[PATCH] 4/9] Documentation-DTbindings-DMA-controller-of-AST2500
Signed-off-by: sudheer.v
---
.../devicetree/bindings/dma/ast-uart-sdma.txt | 23 ++
1 file changed, 23 insertions(+)
create mode 100644 Documentation/devicetree/bindings/dma/ast-uart-sdma.txt
diff --git a/Documentation/devicetree/bindings/dma/ast-uart-sdma.txt
b/Documentation/devicetree/bindings/dma/ast-uart-sdma.txt
new file mode 100644
index 000..e770df2
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/ast-uart-sdma.txt
@@ -0,0 +1,23 @@
+* Aspeed Direct Memory Access (DMA) Controller for AST25XX
+
+
+* DMA controller
+
+Required properties:
+- compatible : Should be "aspeed,ast-uart-sdma"
+- reg : Should contain SDMA registers location and length
+- interrupts : Should contain SDMA interrupt
+- dma-channels: number of DMA channels in DMA controller
+
+Optional properties:
+
+Example
+ast_uart_sdma: uart_sdma@1e79e000 {
+compatible = "aspeed,ast-uart-sdma";
+reg = <0x1e79e000 0x400>;
+interrupts = <50>;
+#dma-cells = <1>;
+dma-channels = <8>;
+status = "disabled";
+};
+
--
1.9.1
[[PATCH] 2/9] Defconfig-changes-for-DMA-UART-of-AST2500
Signed-off-by: sudheer.v --- arch/arm/configs/aspeed_g5_defconfig | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig index b7f8fa1..25813b5 100644 --- a/arch/arm/configs/aspeed_g5_defconfig +++ b/arch/arm/configs/aspeed_g5_defconfig @@ -132,8 +132,8 @@ CONFIG_KEYBOARD_GPIO_POLLED=y CONFIG_SERIAL_8250=y # CONFIG_SERIAL_8250_DEPRECATED_OPTIONS is not set CONFIG_SERIAL_8250_CONSOLE=y -CONFIG_SERIAL_8250_NR_UARTS=6 -CONFIG_SERIAL_8250_RUNTIME_UARTS=6 +CONFIG_SERIAL_8250_NR_UARTS=1 +CONFIG_SERIAL_8250_RUNTIME_UARTS=1 CONFIG_SERIAL_8250_EXTENDED=y CONFIG_SERIAL_8250_ASPEED_VUART=y CONFIG_SERIAL_8250_SHARE_IRQ=y @@ -211,6 +211,8 @@ CONFIG_RTC_CLASS=y CONFIG_RTC_DRV_DS1307=y CONFIG_RTC_DRV_PCF8523=y CONFIG_RTC_DRV_RV8803=y +CONFIG_DMADEVICES=y +CONFIG_AST_UART_SDMA=y # CONFIG_VIRTIO_MENU is not set # CONFIG_IOMMU_SUPPORT is not set CONFIG_IIO=y -- 1.9.1
[[PATCH] 8/9] DMA-UART-Driver-for-AST2500
Signed-off-by: sudheer.v
---
drivers/tty/serial/8250/8250_aspeed_uart_dma.c | 1594
1 file changed, 1594 insertions(+)
create mode 100644 drivers/tty/serial/8250/8250_aspeed_uart_dma.c
diff --git a/drivers/tty/serial/8250/8250_aspeed_uart_dma.c
b/drivers/tty/serial/8250/8250_aspeed_uart_dma.c
new file mode 100644
index 000..e1019a8
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_aspeed_uart_dma.c
@@ -0,0 +1,1594 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * drivers/tty/serial/8250/8250_aspeed_uart_dma.c
+ *1. 2018/07/01 Shivah Shankar created
+ *2. 2018/08/25 sudheer.veliseti modified
+ *
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "8250.h"
+#include
+#define SDMA_RX_BUFF_SIZE 0x1 //65536
+#define DMA_BUFF_SIZE 0x1000 //4096
+
+
+
+
+#undef UART_XMIT_SIZE
+#define UART_XMIT_SIZE 0x1000
+#define UART_RX_SIZE 0x1
+
+#ifdef UART_DMA_DEBUG
+ #define UART_DBG(fmt, args...) pr_debug("%s() " fmt, __func__, ## args)
+#else
+ #define UART_DBG(fmt, args...)
+#endif
+
+#ifdef CONFIG_UART_TX_DMA_DEBUG
+ #define UART_TX_DBG(fmt, args...) pr_debug("%s()"fmt, __func__, ## args)
+#else
+ #define UART_TX_DBG(fmt, args...)
+#endif
+
+/*
+ * Configuration:
+ * share_irqs - whether we pass IRQF_SHARED to request_irq(). This option
+ *is unsafe when used on edge-triggered interrupts.
+ */
+static unsigned int share_irqs = SERIAL8250_SHARE_IRQS;
+
+static unsigned int nr_uarts = CONFIG_AST_RUNTIME_DMA_UARTS;
+
+/*
+ * Debugging.
+ */
+#if 0
+#define DEBUG_AUTOCONF(fmt...) UART_DBG(fmt)
+#else
+#define DEBUG_AUTOCONF(fmt...) do { } while (0)
+#endif
+
+#if 0
+#define DEBUG_INTR(fmt...) UART_DBG(fmt)
+#else
+#define DEBUG_INTR(fmt...) do { } while (0)
+#endif
+
+#define PASS_LIMIT 256
+
+#include
+
+
+#define UART_DMA_NRCONFIG_AST_NR_DMA_UARTS
+
+struct ast_uart_port {
+ struct uart_portport;
+ struct platform_device *pdev;
+ unsigned short capabilities; /* port capabilities */
+ unsigned short bugs; /* port bugs */
+ unsigned inttx_loadsz; /* transmit fifo load size */
+ unsigned char acr;
+ unsigned char ier;
+ unsigned char lcr;
+ unsigned char mcr;
+ unsigned char mcr_mask; /* mask of user bits */
+ unsigned char mcr_force; /* mask of forced bits */
+ unsigned intchannel_no;
+ struct scatterlist rx_sgl;
+ struct dma_chan *rx_dma_chan;
+ struct circ_buf rx_dma_buf;
+ dma_addr_t dma_rx_addr;
+ u8 rx_in_progress;
+ struct dma_async_tx_descriptor *rx_dma_desc;
+ dma_cookie_trx_cookie;
+ unsigned intrx_bytes_requested;
+ unsigned intrx_bytes_transferred;
+ struct tasklet_struct rx_tasklet;
+ struct scatterlist tx_sgl;
+ struct dma_chan *tx_dma_chan;
+ struct circ_buf tx_dma_buf;
+ dma_addr_t dma_tx_addr;
+ u8 tx_in_progress;
+ struct dma_async_tx_descriptor *tx_dma_desc;
+ dma_cookie_ttx_cookie;
+ unsigned inttx_bytes_requested;
+ unsigned inttx_bytes_transferred;
+ struct tasklet_struct tx_tasklet;
+ spinlock_t lock;
+ int tx_done;
+ int tx_count;
+ /*
+* Some bits in registers are cleared on a read, so they must
+* be saved whenever the register is read but the bits will not
+* be immediately processed.
+*/
+#define LSR_SAVE_FLAGS UART_LSR_BRK_ERROR_BITS
+ unsigned char lsr_saved_flags;
+#define MSR_SAVE_FLAGS UART_MSR_ANY_DELTA
+ unsigned char msr_saved_flags;
+
+ /*
+* We provide a per-port pm hook.
+*/
+ void(*pm)(struct uart_port *port,
+ unsigned int state, unsigned int old);
+};
+
+static struct ast_uart_port ast_uart_ports[UART_DMA_NR];
+
+static int ast_dma_channel_setup(struct ast_uart_port *up);
+static inline struct ast_uart_port *
+to_ast_dma_uart_port(struct uart_port *uart)
+{
+ return container_of(uart, struct ast_uart_port, port);
+}
+
+struct irq_info {
+ spinlock_t lock;
+ struct ast_uart_port *up;
+};
+
+static void ast_dma_channel_teardown(struct ast_uart_port *s);
+static struct irq_info ast_uart_irq[1];
+static DEFINE_MUTEX(ast_uart_mutex);
+
+/*
+ * Here we define
[[PATCH] 2/9] Defconfig-changes-for-DMA-UART-of-AST2500
Signed-off-by: sudheer.v --- arch/arm/configs/aspeed_g5_defconfig | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig index b7f8fa1..25813b5 100644 --- a/arch/arm/configs/aspeed_g5_defconfig +++ b/arch/arm/configs/aspeed_g5_defconfig @@ -132,8 +132,8 @@ CONFIG_KEYBOARD_GPIO_POLLED=y CONFIG_SERIAL_8250=y # CONFIG_SERIAL_8250_DEPRECATED_OPTIONS is not set CONFIG_SERIAL_8250_CONSOLE=y -CONFIG_SERIAL_8250_NR_UARTS=6 -CONFIG_SERIAL_8250_RUNTIME_UARTS=6 +CONFIG_SERIAL_8250_NR_UARTS=1 +CONFIG_SERIAL_8250_RUNTIME_UARTS=1 CONFIG_SERIAL_8250_EXTENDED=y CONFIG_SERIAL_8250_ASPEED_VUART=y CONFIG_SERIAL_8250_SHARE_IRQ=y @@ -211,6 +211,8 @@ CONFIG_RTC_CLASS=y CONFIG_RTC_DRV_DS1307=y CONFIG_RTC_DRV_PCF8523=y CONFIG_RTC_DRV_RV8803=y +CONFIG_DMADEVICES=y +CONFIG_AST_UART_SDMA=y # CONFIG_VIRTIO_MENU is not set # CONFIG_IOMMU_SUPPORT is not set CONFIG_IIO=y -- 1.9.1
[[PATCH] 8/9] DMA-UART-Driver-for-AST2500
Signed-off-by: sudheer.v
---
drivers/tty/serial/8250/8250_aspeed_uart_dma.c | 1594
1 file changed, 1594 insertions(+)
create mode 100644 drivers/tty/serial/8250/8250_aspeed_uart_dma.c
diff --git a/drivers/tty/serial/8250/8250_aspeed_uart_dma.c
b/drivers/tty/serial/8250/8250_aspeed_uart_dma.c
new file mode 100644
index 000..e1019a8
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_aspeed_uart_dma.c
@@ -0,0 +1,1594 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * drivers/tty/serial/8250/8250_aspeed_uart_dma.c
+ *1. 2018/07/01 Shivah Shankar created
+ *2. 2018/08/25 sudheer.veliseti modified
+ *
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "8250.h"
+#include
+#define SDMA_RX_BUFF_SIZE 0x1 //65536
+#define DMA_BUFF_SIZE 0x1000 //4096
+
+
+
+
+#undef UART_XMIT_SIZE
+#define UART_XMIT_SIZE 0x1000
+#define UART_RX_SIZE 0x1
+
+#ifdef UART_DMA_DEBUG
+ #define UART_DBG(fmt, args...) pr_debug("%s() " fmt, __func__, ## args)
+#else
+ #define UART_DBG(fmt, args...)
+#endif
+
+#ifdef CONFIG_UART_TX_DMA_DEBUG
+ #define UART_TX_DBG(fmt, args...) pr_debug("%s()"fmt, __func__, ## args)
+#else
+ #define UART_TX_DBG(fmt, args...)
+#endif
+
+/*
+ * Configuration:
+ * share_irqs - whether we pass IRQF_SHARED to request_irq(). This option
+ *is unsafe when used on edge-triggered interrupts.
+ */
+static unsigned int share_irqs = SERIAL8250_SHARE_IRQS;
+
+static unsigned int nr_uarts = CONFIG_AST_RUNTIME_DMA_UARTS;
+
+/*
+ * Debugging.
+ */
+#if 0
+#define DEBUG_AUTOCONF(fmt...) UART_DBG(fmt)
+#else
+#define DEBUG_AUTOCONF(fmt...) do { } while (0)
+#endif
+
+#if 0
+#define DEBUG_INTR(fmt...) UART_DBG(fmt)
+#else
+#define DEBUG_INTR(fmt...) do { } while (0)
+#endif
+
+#define PASS_LIMIT 256
+
+#include
+
+
+#define UART_DMA_NRCONFIG_AST_NR_DMA_UARTS
+
+struct ast_uart_port {
+ struct uart_portport;
+ struct platform_device *pdev;
+ unsigned short capabilities; /* port capabilities */
+ unsigned short bugs; /* port bugs */
+ unsigned inttx_loadsz; /* transmit fifo load size */
+ unsigned char acr;
+ unsigned char ier;
+ unsigned char lcr;
+ unsigned char mcr;
+ unsigned char mcr_mask; /* mask of user bits */
+ unsigned char mcr_force; /* mask of forced bits */
+ unsigned intchannel_no;
+ struct scatterlist rx_sgl;
+ struct dma_chan *rx_dma_chan;
+ struct circ_buf rx_dma_buf;
+ dma_addr_t dma_rx_addr;
+ u8 rx_in_progress;
+ struct dma_async_tx_descriptor *rx_dma_desc;
+ dma_cookie_trx_cookie;
+ unsigned intrx_bytes_requested;
+ unsigned intrx_bytes_transferred;
+ struct tasklet_struct rx_tasklet;
+ struct scatterlist tx_sgl;
+ struct dma_chan *tx_dma_chan;
+ struct circ_buf tx_dma_buf;
+ dma_addr_t dma_tx_addr;
+ u8 tx_in_progress;
+ struct dma_async_tx_descriptor *tx_dma_desc;
+ dma_cookie_ttx_cookie;
+ unsigned inttx_bytes_requested;
+ unsigned inttx_bytes_transferred;
+ struct tasklet_struct tx_tasklet;
+ spinlock_t lock;
+ int tx_done;
+ int tx_count;
+ /*
+* Some bits in registers are cleared on a read, so they must
+* be saved whenever the register is read but the bits will not
+* be immediately processed.
+*/
+#define LSR_SAVE_FLAGS UART_LSR_BRK_ERROR_BITS
+ unsigned char lsr_saved_flags;
+#define MSR_SAVE_FLAGS UART_MSR_ANY_DELTA
+ unsigned char msr_saved_flags;
+
+ /*
+* We provide a per-port pm hook.
+*/
+ void(*pm)(struct uart_port *port,
+ unsigned int state, unsigned int old);
+};
+
+static struct ast_uart_port ast_uart_ports[UART_DMA_NR];
+
+static int ast_dma_channel_setup(struct ast_uart_port *up);
+static inline struct ast_uart_port *
+to_ast_dma_uart_port(struct uart_port *uart)
+{
+ return container_of(uart, struct ast_uart_port, port);
+}
+
+struct irq_info {
+ spinlock_t lock;
+ struct ast_uart_port *up;
+};
+
+static void ast_dma_channel_teardown(struct ast_uart_port *s);
+static struct irq_info ast_uart_irq[1];
+static DEFINE_MUTEX(ast_uart_mutex);
+
+/*
+ * Here we define
[[PATCH] 4/9] Documentation-DTbindings-DMA-controller-of-AST2500
Signed-off-by: sudheer.v
---
.../devicetree/bindings/dma/ast-uart-sdma.txt | 23 ++
1 file changed, 23 insertions(+)
create mode 100644 Documentation/devicetree/bindings/dma/ast-uart-sdma.txt
diff --git a/Documentation/devicetree/bindings/dma/ast-uart-sdma.txt
b/Documentation/devicetree/bindings/dma/ast-uart-sdma.txt
new file mode 100644
index 000..e770df2
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/ast-uart-sdma.txt
@@ -0,0 +1,23 @@
+* Aspeed Direct Memory Access (DMA) Controller for AST25XX
+
+
+* DMA controller
+
+Required properties:
+- compatible : Should be "aspeed,ast-uart-sdma"
+- reg : Should contain SDMA registers location and length
+- interrupts : Should contain SDMA interrupt
+- dma-channels: number of DMA channels in DMA controller
+
+Optional properties:
+
+Example
+ast_uart_sdma: uart_sdma@1e79e000 {
+compatible = "aspeed,ast-uart-sdma";
+reg = <0x1e79e000 0x400>;
+interrupts = <50>;
+#dma-cells = <1>;
+dma-channels = <8>;
+status = "disabled";
+};
+
--
1.9.1
[[PATCH] 0/9] *** DMA support for UART in ASPEED's AST2500 ***
DMA controller driver and UART dma client driver for aspeed's AST2500 sudheer.v (9): DT-changes-for-DMA-UART-of-AST2500 Defconfig-changes-for-DMA-UART-of-AST2500 configuration-for-DMA-of-AST2500 Documentation-DTbindings-DMA-controller-of-AST2500 DMA-driver-for-AST2500 configuration-for-DMA-UART-of-AST2500 Documentation-DTbindings-DMA-UART-of-AST2500 DMA-UART-Driver-for-AST2500 updating MAINTAINERS for DMA and DMA-UART drivers of AST2500 .../devicetree/bindings/dma/ast-uart-sdma.txt | 23 + .../devicetree/bindings/serial/ast-sdma-uart.txt | 34 + MAINTAINERS| 26 + arch/arm/boot/dts/aspeed-ast2500-evb.dts | 20 + arch/arm/boot/dts/aspeed-g5.dtsi | 85 ++ arch/arm/configs/aspeed_g5_defconfig |6 +- drivers/dma/Kconfig|9 + drivers/dma/Makefile |1 + drivers/dma/aspeed_uart_sdma.c | 810 ++ drivers/tty/serial/8250/8250_aspeed_uart_dma.c | 1594 drivers/tty/serial/8250/Kconfig| 34 + drivers/tty/serial/8250/Makefile |1 + 12 files changed, 2641 insertions(+), 2 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/ast-uart-sdma.txt create mode 100644 Documentation/devicetree/bindings/serial/ast-sdma-uart.txt create mode 100644 drivers/dma/aspeed_uart_sdma.c create mode 100644 drivers/tty/serial/8250/8250_aspeed_uart_dma.c -- 1.9.1
[[PATCH] 0/9] *** DMA support for UART in ASPEED's AST2500 ***
DMA controller driver and UART dma client driver for aspeed's AST2500 sudheer.v (9): DT-changes-for-DMA-UART-of-AST2500 Defconfig-changes-for-DMA-UART-of-AST2500 configuration-for-DMA-of-AST2500 Documentation-DTbindings-DMA-controller-of-AST2500 DMA-driver-for-AST2500 configuration-for-DMA-UART-of-AST2500 Documentation-DTbindings-DMA-UART-of-AST2500 DMA-UART-Driver-for-AST2500 updating MAINTAINERS for DMA and DMA-UART drivers of AST2500 .../devicetree/bindings/dma/ast-uart-sdma.txt | 23 + .../devicetree/bindings/serial/ast-sdma-uart.txt | 34 + MAINTAINERS| 26 + arch/arm/boot/dts/aspeed-ast2500-evb.dts | 20 + arch/arm/boot/dts/aspeed-g5.dtsi | 85 ++ arch/arm/configs/aspeed_g5_defconfig |6 +- drivers/dma/Kconfig|9 + drivers/dma/Makefile |1 + drivers/dma/aspeed_uart_sdma.c | 810 ++ drivers/tty/serial/8250/8250_aspeed_uart_dma.c | 1594 drivers/tty/serial/8250/Kconfig| 34 + drivers/tty/serial/8250/Makefile |1 + 12 files changed, 2641 insertions(+), 2 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/ast-uart-sdma.txt create mode 100644 Documentation/devicetree/bindings/serial/ast-sdma-uart.txt create mode 100644 drivers/dma/aspeed_uart_sdma.c create mode 100644 drivers/tty/serial/8250/8250_aspeed_uart_dma.c -- 1.9.1
[[PATCH] 1/9] DT-changes-for-DMA-UART-of-AST2500
Signed-off-by: sudheer.v
---
arch/arm/boot/dts/aspeed-ast2500-evb.dts | 20
arch/arm/boot/dts/aspeed-g5.dtsi | 85
2 files changed, 105 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-ast2500-evb.dts
b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
index 5dbb33c..f98d55b 100644
--- a/arch/arm/boot/dts/aspeed-ast2500-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
@@ -64,6 +64,26 @@
status = "okay";
};
+_uart_sdma{
+ status = "okay";
+};
+
+_uart0 {
+ status = "disabled";
+};
+
+_uart1 {
+ status = "disabled";
+};
+
+_uart2 {
+ status = "okay";
+};
+
+_uart3 {
+ status = "okay";
+};
+
{
status = "okay";
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index d92f047..ba8edd1 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -436,6 +436,91 @@
status = "disabled";
};
+ ast_uart_sdma: uart_sdma@1e79e000 {
+ compatible = "aspeed,ast-uart-sdma";
+ reg = <0x1e79e000 0x400>;
+ interrupts = <50>;
+ #dma-cells = <1>;
+ dma-channels = <8>;
+ status = "disabled";
+ };
+
+ dma_uart0: dma_uart0@1e783000{
+ compatible = "aspeed,ast-sdma-uart";
+ reg = <0x1e783000 0x1000>;
+ interrupts = <9>;
+ clocks = < ASPEED_CLK_GATE_UART1CLK>;
+ reg-shift = <2>;
+ dma-channel = <0>;
+ no-loopback-test;
+ pinctrl-names = "default";
+ pinctrl-0 = <_txd1_default
+ _rxd1_default _ncts1_default
+ _ndcd1_default _ndsr1_default
+ _ndtr1_default _nri1_default
+ _nrts1_default>;
+ dma-names = "rx", "tx";
+ dmas = <_uart_sdma 1>, <_uart_sdma 0>;
+ status = "disabled";
+ };
+
+ dma_uart1: dma_uart1@1e78d000{
+ compatible = "aspeed,ast-sdma-uart";
+ reg = <0x1e78d000 0x1000>;
+ interrupts = <32>;
+ clocks = < ASPEED_CLK_GATE_UART2CLK>;
+ reg-shift = <2>;
+ dma-channel = <1>;
+ no-loopback-test;
+ pinctrl-names = "default";
+ pinctrl-0 = <_txd2_default
+ _rxd2_default _ncts2_default
+ _ndcd2_default _ndsr2_default
+ _ndtr2_default _nri2_default
+ _nrts2_default>;
+ dma-names = "rx", "tx";
+ dmas = <_uart_sdma 3>, <_uart_sdma 2>;
+ status = "disabled";
+};
+
+ dma_uart2: dma_uart2@1e78e000{
+ compatible = "aspeed,ast-sdma-uart";
+ reg = <0x1e78e000 0x1000>;
+ interrupts = <33>;
+ clocks = < ASPEED_CLK_GATE_UART3CLK>;
+ reg-shift = <2>;
+ dma-channel = <2>;
+ no-loopback-test;
+ pinctrl-names = "default";
+ pinctrl-0 = <_txd3_default
+ _rxd3_default _ncts3_default
+ _ndcd3_default _ndsr3_default
+ _ndtr3_default _nri3_default
+ _nrts3_default>;
+ dma-names = "rx", "tx";
+ dmas = <_uart_sdma 5>, <_uart_sdma 4>;
+ status = "disabled";
+};
+
+ dma_uart3: dma_uart3@1e78f000{
+ compatible = "aspeed,ast-sdma-uart";
+ reg = <0x1e78f000 0x1000>;
+ interrupts = <34>;
+ clocks = < ASPEED_CLK_GATE_UART4CLK>;
+ reg-shift = <2>;
+ dma-channel = <3>;
+ no-loopback-test;
+ pinctrl-names = "default";
+ pinctrl-0 =
[[PATCH] 1/9] DT-changes-for-DMA-UART-of-AST2500
Signed-off-by: sudheer.v
---
arch/arm/boot/dts/aspeed-ast2500-evb.dts | 20
arch/arm/boot/dts/aspeed-g5.dtsi | 85
2 files changed, 105 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-ast2500-evb.dts
b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
index 5dbb33c..f98d55b 100644
--- a/arch/arm/boot/dts/aspeed-ast2500-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
@@ -64,6 +64,26 @@
status = "okay";
};
+_uart_sdma{
+ status = "okay";
+};
+
+_uart0 {
+ status = "disabled";
+};
+
+_uart1 {
+ status = "disabled";
+};
+
+_uart2 {
+ status = "okay";
+};
+
+_uart3 {
+ status = "okay";
+};
+
{
status = "okay";
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index d92f047..ba8edd1 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -436,6 +436,91 @@
status = "disabled";
};
+ ast_uart_sdma: uart_sdma@1e79e000 {
+ compatible = "aspeed,ast-uart-sdma";
+ reg = <0x1e79e000 0x400>;
+ interrupts = <50>;
+ #dma-cells = <1>;
+ dma-channels = <8>;
+ status = "disabled";
+ };
+
+ dma_uart0: dma_uart0@1e783000{
+ compatible = "aspeed,ast-sdma-uart";
+ reg = <0x1e783000 0x1000>;
+ interrupts = <9>;
+ clocks = < ASPEED_CLK_GATE_UART1CLK>;
+ reg-shift = <2>;
+ dma-channel = <0>;
+ no-loopback-test;
+ pinctrl-names = "default";
+ pinctrl-0 = <_txd1_default
+ _rxd1_default _ncts1_default
+ _ndcd1_default _ndsr1_default
+ _ndtr1_default _nri1_default
+ _nrts1_default>;
+ dma-names = "rx", "tx";
+ dmas = <_uart_sdma 1>, <_uart_sdma 0>;
+ status = "disabled";
+ };
+
+ dma_uart1: dma_uart1@1e78d000{
+ compatible = "aspeed,ast-sdma-uart";
+ reg = <0x1e78d000 0x1000>;
+ interrupts = <32>;
+ clocks = < ASPEED_CLK_GATE_UART2CLK>;
+ reg-shift = <2>;
+ dma-channel = <1>;
+ no-loopback-test;
+ pinctrl-names = "default";
+ pinctrl-0 = <_txd2_default
+ _rxd2_default _ncts2_default
+ _ndcd2_default _ndsr2_default
+ _ndtr2_default _nri2_default
+ _nrts2_default>;
+ dma-names = "rx", "tx";
+ dmas = <_uart_sdma 3>, <_uart_sdma 2>;
+ status = "disabled";
+};
+
+ dma_uart2: dma_uart2@1e78e000{
+ compatible = "aspeed,ast-sdma-uart";
+ reg = <0x1e78e000 0x1000>;
+ interrupts = <33>;
+ clocks = < ASPEED_CLK_GATE_UART3CLK>;
+ reg-shift = <2>;
+ dma-channel = <2>;
+ no-loopback-test;
+ pinctrl-names = "default";
+ pinctrl-0 = <_txd3_default
+ _rxd3_default _ncts3_default
+ _ndcd3_default _ndsr3_default
+ _ndtr3_default _nri3_default
+ _nrts3_default>;
+ dma-names = "rx", "tx";
+ dmas = <_uart_sdma 5>, <_uart_sdma 4>;
+ status = "disabled";
+};
+
+ dma_uart3: dma_uart3@1e78f000{
+ compatible = "aspeed,ast-sdma-uart";
+ reg = <0x1e78f000 0x1000>;
+ interrupts = <34>;
+ clocks = < ASPEED_CLK_GATE_UART4CLK>;
+ reg-shift = <2>;
+ dma-channel = <3>;
+ no-loopback-test;
+ pinctrl-names = "default";
+ pinctrl-0 =
[PATCH] perf: Add Sparc annotate support.
Signed-off-by: David S. Miller
---
tools/perf/arch/sparc/annotate/instructions.c | 169 ++
tools/perf/util/annotate.c| 8 +
2 files changed, 177 insertions(+)
create mode 100644 tools/perf/arch/sparc/annotate/instructions.c
diff --git a/tools/perf/arch/sparc/annotate/instructions.c
b/tools/perf/arch/sparc/annotate/instructions.c
new file mode 100644
index ..2614c010c235
--- /dev/null
+++ b/tools/perf/arch/sparc/annotate/instructions.c
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0
+
+static int is_branch_cond(const char *cond)
+{
+ if (cond[0] == '\0')
+ return 1;
+
+ if (cond[0] == 'a' && cond[1] == '\0')
+ return 1;
+
+ if (cond[0] == 'c' &&
+ (cond[1] == 'c' || cond[1] == 's') &&
+ cond[2] == '\0')
+ return 1;
+
+ if (cond[0] == 'e' &&
+ (cond[1] == '\0' ||
+(cond[1] == 'q' && cond[2] == '\0')))
+ return 1;
+
+ if (cond[0] == 'g' &&
+ (cond[1] == '\0' ||
+(cond[1] == 't' && cond[2] == '\0') ||
+(cond[1] == 'e' && cond[2] == '\0') ||
+(cond[1] == 'e' && cond[2] == 'u' && cond[3] == '\0')))
+ return 1;
+
+ if (cond[0] == 'l' &&
+ (cond[1] == '\0' ||
+(cond[1] == 't' && cond[2] == '\0') ||
+(cond[1] == 'u' && cond[2] == '\0') ||
+(cond[1] == 'e' && cond[2] == '\0') ||
+(cond[1] == 'e' && cond[2] == 'u' && cond[3] == '\0')))
+ return 1;
+
+ if (cond[0] == 'n' &&
+ (cond[1] == '\0' ||
+(cond[1] == 'e' && cond[2] == '\0') ||
+(cond[1] == 'z' && cond[2] == '\0') ||
+(cond[1] == 'e' && cond[2] == 'g' && cond[3] == '\0')))
+ return 1;
+
+ if (cond[0] == 'b' &&
+ cond[1] == 'p' &&
+ cond[2] == 'o' &&
+ cond[3] == 's' &&
+ cond[4] == '\0')
+ return 1;
+
+ if (cond[0] == 'v' &&
+ (cond[1] == 'c' || cond[1] == 's') &&
+ cond[2] == '\0')
+ return 1;
+
+ if (cond[0] == 'b' &&
+ cond[1] == 'z' &&
+ cond[2] == '\0')
+ return 1;
+
+ return 0;
+}
+
+static int is_branch_reg_cond(const char *cond)
+{
+ if ((cond[0] == 'n' || cond[0] == 'l') &&
+ cond[1] == 'z' &&
+ cond[2] == '\0')
+ return 1;
+
+ if (cond[0] == 'z' &&
+ cond[1] == '\0')
+ return 1;
+
+ if ((cond[0] == 'g' || cond[0] == 'l') &&
+ cond[1] == 'e' &&
+ cond[2] == 'z' &&
+ cond[3] == '\0')
+ return 1;
+
+ if (cond[0] == 'g' &&
+ cond[1] == 'z' &&
+ cond[2] == '\0')
+ return 1;
+
+ return 0;
+}
+
+static int is_branch_float_cond(const char *cond)
+{
+ if (cond[0] == '\0')
+ return 1;
+
+ if ((cond[0] == 'a' || cond[0] == 'e' ||
+cond[0] == 'z' || cond[0] == 'g' ||
+cond[0] == 'l' || cond[0] == 'n' ||
+cond[0] == 'o' || cond[0] == 'u') &&
+ cond[1] == '\0')
+ return 1;
+
+ if (((cond[0] == 'g' && cond[1] == 'e') ||
+(cond[0] == 'l' && (cond[1] == 'e' ||
+cond[1] == 'g')) ||
+(cond[0] == 'n' && (cond[1] == 'e' ||
+cond[1] == 'z')) ||
+(cond[0] == 'u' && (cond[1] == 'e' ||
+cond[1] == 'g' ||
+cond[1] == 'l'))) &&
+ cond[2] == '\0')
+ return 1;
+
+ if (cond[0] == 'u' &&
+ (cond[1] == 'g' || cond[1] == 'l') &&
+ cond[2] == 'e' &&
+ cond[3] == '\0')
+ return 1;
+
+ return 0;
+}
+
+static struct ins_ops *sparc__associate_instruction_ops(struct arch *arch,
const char *name)
+{
+ struct ins_ops *ops = NULL;
+
+ if (!strcmp(name, "call") ||
+ !strcmp(name, "jmp") ||
+ !strcmp(name, "jmpl")) {
+ ops = _ops;
+ } else if (!strcmp(name, "ret") ||
+ !strcmp(name, "retl") ||
+ !strcmp(name, "return")) {
+ ops = _ops;
+ } else if (!strcmp(name, "mov")) {
+ ops = _ops;
+ } else {
+ if (name[0] == 'c' &&
+ (name[1] == 'w' || name[1] == 'x'))
+ name += 2;
+
+ if (name[0] == 'b') {
+ const char *cond = name + 1;
+
+ if (cond[0] == 'r') {
+ if (is_branch_reg_cond(cond + 1))
+ ops = _ops;
+ } else if (is_branch_cond(cond)) {
+ ops = _ops;
+ }
+ } else if (name[0] == 'f'
Re: [PATCH 4.14 000/109] 4.14.77-stable review
On Tue, Oct 16, 2018 at 07:04:28PM +0200, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 4.14.77 release. > There are 109 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Thu Oct 18 17:04:58 UTC 2018. > Anything received after that time might be too late. Results from Linaro’s test farm. No regressions on arm64, arm, x86_64, and i386. Summary kernel: 4.14.77-rc1 git repo: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git git branch: linux-4.14.y git commit: 3dbba66c8671a97270f35e072c54f74ddca6954e git describe: v4.14.76-110-g3dbba66c8671 Test details: https://qa-reports.linaro.org/lkft/linux-stable-rc-4.14-oe/build/v4.14.76-110-g3dbba66c8671 No regressions (compared to build v4.14.76) No fixes (compared to build v4.14.76) Ran 21021 total tests in the following environments and test suites. Environments -- - dragonboard-410c - arm64 - hi6220-hikey - arm64 - i386 - juno-r2 - arm64 - qemu_arm - qemu_arm64 - qemu_i386 - qemu_x86_64 - x15 - arm - x86_64 Test Suites --- * boot * libhugetlbfs * ltp-containers-tests * ltp-cve-tests * ltp-fcntl-locktests-tests * ltp-filecaps-tests * ltp-fs-tests * ltp-fs_bind-tests * ltp-fs_perms_simple-tests * ltp-fsx-tests * ltp-hugetlb-tests * ltp-io-tests * ltp-ipc-tests * ltp-math-tests * ltp-nptl-tests * ltp-pty-tests * ltp-sched-tests * ltp-securebits-tests * ltp-syscalls-tests * ltp-timers-tests * kselftest * ltp-cap_bounds-tests * ltp-open-posix-tests * kselftest-vsyscall-mode-native * kselftest-vsyscall-mode-none -- Linaro LKFT https://lkft.linaro.org
Re: [PATCH 4.14 000/109] 4.14.77-stable review
On Tue, Oct 16, 2018 at 07:04:28PM +0200, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 4.14.77 release. > There are 109 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Thu Oct 18 17:04:58 UTC 2018. > Anything received after that time might be too late. Results from Linaro’s test farm. No regressions on arm64, arm, x86_64, and i386. Summary kernel: 4.14.77-rc1 git repo: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git git branch: linux-4.14.y git commit: 3dbba66c8671a97270f35e072c54f74ddca6954e git describe: v4.14.76-110-g3dbba66c8671 Test details: https://qa-reports.linaro.org/lkft/linux-stable-rc-4.14-oe/build/v4.14.76-110-g3dbba66c8671 No regressions (compared to build v4.14.76) No fixes (compared to build v4.14.76) Ran 21021 total tests in the following environments and test suites. Environments -- - dragonboard-410c - arm64 - hi6220-hikey - arm64 - i386 - juno-r2 - arm64 - qemu_arm - qemu_arm64 - qemu_i386 - qemu_x86_64 - x15 - arm - x86_64 Test Suites --- * boot * libhugetlbfs * ltp-containers-tests * ltp-cve-tests * ltp-fcntl-locktests-tests * ltp-filecaps-tests * ltp-fs-tests * ltp-fs_bind-tests * ltp-fs_perms_simple-tests * ltp-fsx-tests * ltp-hugetlb-tests * ltp-io-tests * ltp-ipc-tests * ltp-math-tests * ltp-nptl-tests * ltp-pty-tests * ltp-sched-tests * ltp-securebits-tests * ltp-syscalls-tests * ltp-timers-tests * kselftest * ltp-cap_bounds-tests * ltp-open-posix-tests * kselftest-vsyscall-mode-native * kselftest-vsyscall-mode-none -- Linaro LKFT https://lkft.linaro.org
[PATCH] perf: Add Sparc annotate support.
Signed-off-by: David S. Miller
---
tools/perf/arch/sparc/annotate/instructions.c | 169 ++
tools/perf/util/annotate.c| 8 +
2 files changed, 177 insertions(+)
create mode 100644 tools/perf/arch/sparc/annotate/instructions.c
diff --git a/tools/perf/arch/sparc/annotate/instructions.c
b/tools/perf/arch/sparc/annotate/instructions.c
new file mode 100644
index ..2614c010c235
--- /dev/null
+++ b/tools/perf/arch/sparc/annotate/instructions.c
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0
+
+static int is_branch_cond(const char *cond)
+{
+ if (cond[0] == '\0')
+ return 1;
+
+ if (cond[0] == 'a' && cond[1] == '\0')
+ return 1;
+
+ if (cond[0] == 'c' &&
+ (cond[1] == 'c' || cond[1] == 's') &&
+ cond[2] == '\0')
+ return 1;
+
+ if (cond[0] == 'e' &&
+ (cond[1] == '\0' ||
+(cond[1] == 'q' && cond[2] == '\0')))
+ return 1;
+
+ if (cond[0] == 'g' &&
+ (cond[1] == '\0' ||
+(cond[1] == 't' && cond[2] == '\0') ||
+(cond[1] == 'e' && cond[2] == '\0') ||
+(cond[1] == 'e' && cond[2] == 'u' && cond[3] == '\0')))
+ return 1;
+
+ if (cond[0] == 'l' &&
+ (cond[1] == '\0' ||
+(cond[1] == 't' && cond[2] == '\0') ||
+(cond[1] == 'u' && cond[2] == '\0') ||
+(cond[1] == 'e' && cond[2] == '\0') ||
+(cond[1] == 'e' && cond[2] == 'u' && cond[3] == '\0')))
+ return 1;
+
+ if (cond[0] == 'n' &&
+ (cond[1] == '\0' ||
+(cond[1] == 'e' && cond[2] == '\0') ||
+(cond[1] == 'z' && cond[2] == '\0') ||
+(cond[1] == 'e' && cond[2] == 'g' && cond[3] == '\0')))
+ return 1;
+
+ if (cond[0] == 'b' &&
+ cond[1] == 'p' &&
+ cond[2] == 'o' &&
+ cond[3] == 's' &&
+ cond[4] == '\0')
+ return 1;
+
+ if (cond[0] == 'v' &&
+ (cond[1] == 'c' || cond[1] == 's') &&
+ cond[2] == '\0')
+ return 1;
+
+ if (cond[0] == 'b' &&
+ cond[1] == 'z' &&
+ cond[2] == '\0')
+ return 1;
+
+ return 0;
+}
+
+static int is_branch_reg_cond(const char *cond)
+{
+ if ((cond[0] == 'n' || cond[0] == 'l') &&
+ cond[1] == 'z' &&
+ cond[2] == '\0')
+ return 1;
+
+ if (cond[0] == 'z' &&
+ cond[1] == '\0')
+ return 1;
+
+ if ((cond[0] == 'g' || cond[0] == 'l') &&
+ cond[1] == 'e' &&
+ cond[2] == 'z' &&
+ cond[3] == '\0')
+ return 1;
+
+ if (cond[0] == 'g' &&
+ cond[1] == 'z' &&
+ cond[2] == '\0')
+ return 1;
+
+ return 0;
+}
+
+static int is_branch_float_cond(const char *cond)
+{
+ if (cond[0] == '\0')
+ return 1;
+
+ if ((cond[0] == 'a' || cond[0] == 'e' ||
+cond[0] == 'z' || cond[0] == 'g' ||
+cond[0] == 'l' || cond[0] == 'n' ||
+cond[0] == 'o' || cond[0] == 'u') &&
+ cond[1] == '\0')
+ return 1;
+
+ if (((cond[0] == 'g' && cond[1] == 'e') ||
+(cond[0] == 'l' && (cond[1] == 'e' ||
+cond[1] == 'g')) ||
+(cond[0] == 'n' && (cond[1] == 'e' ||
+cond[1] == 'z')) ||
+(cond[0] == 'u' && (cond[1] == 'e' ||
+cond[1] == 'g' ||
+cond[1] == 'l'))) &&
+ cond[2] == '\0')
+ return 1;
+
+ if (cond[0] == 'u' &&
+ (cond[1] == 'g' || cond[1] == 'l') &&
+ cond[2] == 'e' &&
+ cond[3] == '\0')
+ return 1;
+
+ return 0;
+}
+
+static struct ins_ops *sparc__associate_instruction_ops(struct arch *arch,
const char *name)
+{
+ struct ins_ops *ops = NULL;
+
+ if (!strcmp(name, "call") ||
+ !strcmp(name, "jmp") ||
+ !strcmp(name, "jmpl")) {
+ ops = _ops;
+ } else if (!strcmp(name, "ret") ||
+ !strcmp(name, "retl") ||
+ !strcmp(name, "return")) {
+ ops = _ops;
+ } else if (!strcmp(name, "mov")) {
+ ops = _ops;
+ } else {
+ if (name[0] == 'c' &&
+ (name[1] == 'w' || name[1] == 'x'))
+ name += 2;
+
+ if (name[0] == 'b') {
+ const char *cond = name + 1;
+
+ if (cond[0] == 'r') {
+ if (is_branch_reg_cond(cond + 1))
+ ops = _ops;
+ } else if (is_branch_cond(cond)) {
+ ops = _ops;
+ }
+ } else if (name[0] == 'f'
RE: [PATCH v3 1/2] mtd: spi-nor: add support to non-uniform SFDP SPI NOR flash memories
Hi Tudor,
> -Original Message-
> From: Yogesh Narayan Gaur
> Sent: Wednesday, October 17, 2018 7:38 AM
> To: 'Cyrille Pitchen' ; Tudor Ambarus
> ; marek.va...@gmail.com;
> dw...@infradead.org; computersforpe...@gmail.com;
> boris.brezil...@bootlin.com; rich...@nod.at
> Cc: linux-kernel@vger.kernel.org; nicolas.fe...@microchip.com;
> cyrille.pitc...@microchip.com; linux-...@lists.infradead.org; linux-arm-
> ker...@lists.infradead.org; cristian.bir...@microchip.com
> Subject: RE: [PATCH v3 1/2] mtd: spi-nor: add support to non-uniform SFDP SPI
> NOR flash memories
>
> Hi Tudor,
>
> > -Original Message-
> > From: Cyrille Pitchen [mailto:cyrille.pitc...@wedev4u.fr]
> > Sent: Tuesday, October 16, 2018 10:04 PM
> > To: Tudor Ambarus ; Yogesh Narayan Gaur
> > ; marek.va...@gmail.com;
> > dw...@infradead.org; computersforpe...@gmail.com;
> > boris.brezil...@bootlin.com; rich...@nod.at
> > Cc: linux-kernel@vger.kernel.org; nicolas.fe...@microchip.com;
> > cyrille.pitc...@microchip.com; linux-...@lists.infradead.org;
> > linux-arm- ker...@lists.infradead.org; cristian.bir...@microchip.com
> > Subject: Re: [PATCH v3 1/2] mtd: spi-nor: add support to non-uniform
> > SFDP SPI NOR flash memories
> >
> > Hi Tudor,
> >
> > Le 16/10/2018 à 17:14, Tudor Ambarus a écrit :
> > > Hi, Yogesh,
> > >
> > > On 10/16/2018 12:51 PM, Yogesh Narayan Gaur wrote:
> > >> Hi Tudor,
> > >>
> > >> This patch is breaking the 1-4-4 Read protocol for the spansion
> > >> flash
> > "s25fl512s".
> > >>
> > >> Without this patch read request command for Quad mode, 4-byte
> > >> enable, is
> > coming as 0xEC i.e. SPINOR_OP_READ_1_4_4_4B.
> > >> But after applying this patch, read request command for Quad mode
> > >> is
> > coming as 0x6C i.e. SPINOR_OP_READ_1_1_4_4B.
> > >>
> > >> This flash also supports non-uniform erase.
> > >> Can you please check and provide some suggestion?
> > >
> > > I don't have this memory to test it, but I'll try to help.
> > >
> > > Does s25fl512s support non-uniform erase? I'm looking in
> > > datasheet[1] at JEDEC BFPT table, dwords 8 and 9, page 132/146 and
> > > it looks like it supports just 256KB uniform erase.
> > >
> >
> Actually there is no entry of s25fs512s in current spi-nor.c file.
> For my connected flash part, jedec ID read points to s25fl512s. I have asked
> my
> board team to confirm the name of exact connected flash part.
Cypress connected flash part on my target is S25FS512SAGBHV210.
--
Regards
Yogesh Gaur
> When I check the data sheet of s25fs512s, it also points to the same Jedec ID
> information.
> { "s25fl512s", INFO(0x010220, 0x4d00, 256 * 1024, 256, }
>
> But as stated earlier, if I skip reading SFDP or read using 1-1-1 protocol
> then read
> are always correct.
> For 1-4-4 protocol read are wrong and on further debugging found that Read
> code of 0x6C is being send as opcode instead of 0xEC.
>
> If I revert this patch, reads are working fine.
>
> --
> Regards
> Yogesh Gaur
>
> > s25fS512s supports both uniform and non uniform erase options but
> > s25fL512s is always uniform. L is an old memory part, S is newer.
> >
> > Also, the 8th and 9th WORDs of the Basic Flash Parameter Table alone
> > can't tell you whether or not the memory part can be non uniform.
> > If the memory can be non uniform then the sector erase map table is
> > mandatory, hence when the table is missing you know that your memory
> > part is always uniform.
> >
> > Best regards,
> >
> > Cyrille
> >
> > > Thanks,
> > > ta
> > >
> > > [1]
> > >
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> > >
> >
> cypress.com%2Ffile%2F177971%2Fdownloaddata=02%7C01%7Cyogeshn
> > araya
> > >
> >
> n.gaur%40nxp.com%7C76e7e1555f4a4cda378008d63385480b%7C686ea1d3bc2
> > b4c6f
> > >
> >
> a92cd99c5c301635%7C0%7C0%7C636753044876199155sdata=cioC98EH
> > OGlFbg
> > > XPhoIIJ72K3JrNUnzA1pYhSB9jDwg%3Dreserved=0
> > >
> > >>
> > >> --
> > >> Regards
> > >> Yogesh Gaur
> > >>
> > >>> -Original Message-
> > >>> From: linux-mtd [mailto:linux-mtd-boun...@lists.infradead.org] On
> > >>> Behalf Of Tudor Ambarus
> > >>> Sent: Tuesday, September 11, 2018 9:10 PM
> > >>> To: marek.va...@gmail.com; dw...@infradead.org;
> > >>> computersforpe...@gmail.com; boris.brezil...@bootlin.com;
> > >>> rich...@nod.at
> > >>> Cc: Tudor Ambarus ; linux-
> > >>> ker...@vger.kernel.org; nicolas.fe...@microchip.com;
> > >>> cyrille.pitc...@microchip.com; linux-...@lists.infradead.org;
> > >>> linux-arm- ker...@lists.infradead.org;
> > >>> cristian.bir...@microchip.com
> > >>> Subject: [PATCH v3 1/2] mtd: spi-nor: add support to non-uniform
> > >>> SFDP SPI NOR flash memories
> > >>>
> > >>> Based on Cyrille Pitchen's patch
> > >>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2
> > >>> Fl
> > >>> kml.or
> > >>>
> >
> g%2Flkml%2F2017%2F3%2F22%2F935data=02%7C01%7Cyogeshnarayan.
> > >>>
> >
> gaur%40nxp.com%7C3c782e52b7fd4a8b9af008d617fd5154%7C686ea1d3bc2b4
> > >>>
> >
RE: [PATCH v3 1/2] mtd: spi-nor: add support to non-uniform SFDP SPI NOR flash memories
Hi Tudor,
> -Original Message-
> From: Yogesh Narayan Gaur
> Sent: Wednesday, October 17, 2018 7:38 AM
> To: 'Cyrille Pitchen' ; Tudor Ambarus
> ; marek.va...@gmail.com;
> dw...@infradead.org; computersforpe...@gmail.com;
> boris.brezil...@bootlin.com; rich...@nod.at
> Cc: linux-kernel@vger.kernel.org; nicolas.fe...@microchip.com;
> cyrille.pitc...@microchip.com; linux-...@lists.infradead.org; linux-arm-
> ker...@lists.infradead.org; cristian.bir...@microchip.com
> Subject: RE: [PATCH v3 1/2] mtd: spi-nor: add support to non-uniform SFDP SPI
> NOR flash memories
>
> Hi Tudor,
>
> > -Original Message-
> > From: Cyrille Pitchen [mailto:cyrille.pitc...@wedev4u.fr]
> > Sent: Tuesday, October 16, 2018 10:04 PM
> > To: Tudor Ambarus ; Yogesh Narayan Gaur
> > ; marek.va...@gmail.com;
> > dw...@infradead.org; computersforpe...@gmail.com;
> > boris.brezil...@bootlin.com; rich...@nod.at
> > Cc: linux-kernel@vger.kernel.org; nicolas.fe...@microchip.com;
> > cyrille.pitc...@microchip.com; linux-...@lists.infradead.org;
> > linux-arm- ker...@lists.infradead.org; cristian.bir...@microchip.com
> > Subject: Re: [PATCH v3 1/2] mtd: spi-nor: add support to non-uniform
> > SFDP SPI NOR flash memories
> >
> > Hi Tudor,
> >
> > Le 16/10/2018 à 17:14, Tudor Ambarus a écrit :
> > > Hi, Yogesh,
> > >
> > > On 10/16/2018 12:51 PM, Yogesh Narayan Gaur wrote:
> > >> Hi Tudor,
> > >>
> > >> This patch is breaking the 1-4-4 Read protocol for the spansion
> > >> flash
> > "s25fl512s".
> > >>
> > >> Without this patch read request command for Quad mode, 4-byte
> > >> enable, is
> > coming as 0xEC i.e. SPINOR_OP_READ_1_4_4_4B.
> > >> But after applying this patch, read request command for Quad mode
> > >> is
> > coming as 0x6C i.e. SPINOR_OP_READ_1_1_4_4B.
> > >>
> > >> This flash also supports non-uniform erase.
> > >> Can you please check and provide some suggestion?
> > >
> > > I don't have this memory to test it, but I'll try to help.
> > >
> > > Does s25fl512s support non-uniform erase? I'm looking in
> > > datasheet[1] at JEDEC BFPT table, dwords 8 and 9, page 132/146 and
> > > it looks like it supports just 256KB uniform erase.
> > >
> >
> Actually there is no entry of s25fs512s in current spi-nor.c file.
> For my connected flash part, jedec ID read points to s25fl512s. I have asked
> my
> board team to confirm the name of exact connected flash part.
Cypress connected flash part on my target is S25FS512SAGBHV210.
--
Regards
Yogesh Gaur
> When I check the data sheet of s25fs512s, it also points to the same Jedec ID
> information.
> { "s25fl512s", INFO(0x010220, 0x4d00, 256 * 1024, 256, }
>
> But as stated earlier, if I skip reading SFDP or read using 1-1-1 protocol
> then read
> are always correct.
> For 1-4-4 protocol read are wrong and on further debugging found that Read
> code of 0x6C is being send as opcode instead of 0xEC.
>
> If I revert this patch, reads are working fine.
>
> --
> Regards
> Yogesh Gaur
>
> > s25fS512s supports both uniform and non uniform erase options but
> > s25fL512s is always uniform. L is an old memory part, S is newer.
> >
> > Also, the 8th and 9th WORDs of the Basic Flash Parameter Table alone
> > can't tell you whether or not the memory part can be non uniform.
> > If the memory can be non uniform then the sector erase map table is
> > mandatory, hence when the table is missing you know that your memory
> > part is always uniform.
> >
> > Best regards,
> >
> > Cyrille
> >
> > > Thanks,
> > > ta
> > >
> > > [1]
> > >
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> > >
> >
> cypress.com%2Ffile%2F177971%2Fdownloaddata=02%7C01%7Cyogeshn
> > araya
> > >
> >
> n.gaur%40nxp.com%7C76e7e1555f4a4cda378008d63385480b%7C686ea1d3bc2
> > b4c6f
> > >
> >
> a92cd99c5c301635%7C0%7C0%7C636753044876199155sdata=cioC98EH
> > OGlFbg
> > > XPhoIIJ72K3JrNUnzA1pYhSB9jDwg%3Dreserved=0
> > >
> > >>
> > >> --
> > >> Regards
> > >> Yogesh Gaur
> > >>
> > >>> -Original Message-
> > >>> From: linux-mtd [mailto:linux-mtd-boun...@lists.infradead.org] On
> > >>> Behalf Of Tudor Ambarus
> > >>> Sent: Tuesday, September 11, 2018 9:10 PM
> > >>> To: marek.va...@gmail.com; dw...@infradead.org;
> > >>> computersforpe...@gmail.com; boris.brezil...@bootlin.com;
> > >>> rich...@nod.at
> > >>> Cc: Tudor Ambarus ; linux-
> > >>> ker...@vger.kernel.org; nicolas.fe...@microchip.com;
> > >>> cyrille.pitc...@microchip.com; linux-...@lists.infradead.org;
> > >>> linux-arm- ker...@lists.infradead.org;
> > >>> cristian.bir...@microchip.com
> > >>> Subject: [PATCH v3 1/2] mtd: spi-nor: add support to non-uniform
> > >>> SFDP SPI NOR flash memories
> > >>>
> > >>> Based on Cyrille Pitchen's patch
> > >>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2
> > >>> Fl
> > >>> kml.or
> > >>>
> >
> g%2Flkml%2F2017%2F3%2F22%2F935data=02%7C01%7Cyogeshnarayan.
> > >>>
> >
> gaur%40nxp.com%7C3c782e52b7fd4a8b9af008d617fd5154%7C686ea1d3bc2b4
> > >>>
> >
Re: [PATCH v4 3/4] perf/smmuv3: Add MSI irq support
Hi Shameer, Thank you for the patch! Yet something to improve: [auto build test ERROR on linux-sof-driver/master] [also build test ERROR on v4.19-rc8 next-20181016] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Shameer-Kolothum/arm64-SMMUv3-PMU-driver-with-IORT-support/20181017-063949 base: https://github.com/thesofproject/linux master config: sh-allmodconfig (attached as .config) compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=sh All error/warnings (new ones prefixed by >>): In file included from include/linux/kernel.h:11:0, from include/linux/list.h:9, from include/linux/resource_ext.h:17, from include/linux/acpi.h:26, from drivers//perf/arm_smmuv3_pmu.c:37: drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_counter_set_value': include/linux/bitops.h:7:24: warning: left shift count >= width of type [-Wshift-count-overflow] #define BIT(nr) (1UL << (nr)) ^ drivers//perf/arm_smmuv3_pmu.c:152:31: note: in expansion of macro 'BIT' if (smmu_pmu->counter_mask & BIT(32)) ^~~ drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_counter_get_value': include/linux/bitops.h:7:24: warning: left shift count >= width of type [-Wshift-count-overflow] #define BIT(nr) (1UL << (nr)) ^ drivers//perf/arm_smmuv3_pmu.c:162:31: note: in expansion of macro 'BIT' if (smmu_pmu->counter_mask & BIT(32)) ^~~ drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_free_msis': >> drivers//perf/arm_smmuv3_pmu.c:601:2: error: implicit declaration of >> function 'platform_msi_domain_free_irqs'; did you mean >> 'platform_get_device_id'? [-Werror=implicit-function-declaration] platform_msi_domain_free_irqs(dev); ^ platform_get_device_id drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_setup_msi': >> drivers//perf/arm_smmuv3_pmu.c:632:8: error: implicit declaration of >> function 'platform_msi_domain_alloc_irqs'; did you mean >> 'platform_device_alloc'? [-Werror=implicit-function-declaration] ret = platform_msi_domain_alloc_irqs(dev, 1, smmu_pmu_write_msi_msg); ^~ platform_device_alloc In file included from include/linux/list.h:9:0, from include/linux/resource_ext.h:17, from include/linux/acpi.h:26, from drivers//perf/arm_smmuv3_pmu.c:37: include/linux/msi.h:114:38: error: 'struct device' has no member named 'msi_list' #define dev_to_msi_list(dev) (&(dev)->msi_list) ^ include/linux/kernel.h:961:26: note: in definition of macro 'container_of' void *__mptr = (void *)(ptr); \ ^~~ include/linux/list.h:377:2: note: in expansion of macro 'list_entry' list_entry((ptr)->next, type, member) ^~ include/linux/msi.h:116:2: note: in expansion of macro 'list_first_entry' list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list) ^~~~ include/linux/msi.h:116:19: note: in expansion of macro 'dev_to_msi_list' list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list) ^~~ >> drivers//perf/arm_smmuv3_pmu.c:638:9: note: in expansion of macro >> 'first_msi_entry' desc = first_msi_entry(dev); ^~~ In file included from include/linux/ioport.h:13:0, from include/linux/acpi.h:25, from drivers//perf/arm_smmuv3_pmu.c:37: include/linux/msi.h:114:38: error: 'struct device' has no member named 'msi_list' #define dev_to_msi_list(dev) (&(dev)->msi_list) ^ include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert' bool __cond = !(condition);\ ^ include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^~~ include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert' #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) ^~ include/linux/kernel.h:962:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
Re: [PATCH v4 3/4] perf/smmuv3: Add MSI irq support
Hi Shameer, Thank you for the patch! Yet something to improve: [auto build test ERROR on linux-sof-driver/master] [also build test ERROR on v4.19-rc8 next-20181016] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Shameer-Kolothum/arm64-SMMUv3-PMU-driver-with-IORT-support/20181017-063949 base: https://github.com/thesofproject/linux master config: sh-allmodconfig (attached as .config) compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=sh All error/warnings (new ones prefixed by >>): In file included from include/linux/kernel.h:11:0, from include/linux/list.h:9, from include/linux/resource_ext.h:17, from include/linux/acpi.h:26, from drivers//perf/arm_smmuv3_pmu.c:37: drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_counter_set_value': include/linux/bitops.h:7:24: warning: left shift count >= width of type [-Wshift-count-overflow] #define BIT(nr) (1UL << (nr)) ^ drivers//perf/arm_smmuv3_pmu.c:152:31: note: in expansion of macro 'BIT' if (smmu_pmu->counter_mask & BIT(32)) ^~~ drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_counter_get_value': include/linux/bitops.h:7:24: warning: left shift count >= width of type [-Wshift-count-overflow] #define BIT(nr) (1UL << (nr)) ^ drivers//perf/arm_smmuv3_pmu.c:162:31: note: in expansion of macro 'BIT' if (smmu_pmu->counter_mask & BIT(32)) ^~~ drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_free_msis': >> drivers//perf/arm_smmuv3_pmu.c:601:2: error: implicit declaration of >> function 'platform_msi_domain_free_irqs'; did you mean >> 'platform_get_device_id'? [-Werror=implicit-function-declaration] platform_msi_domain_free_irqs(dev); ^ platform_get_device_id drivers//perf/arm_smmuv3_pmu.c: In function 'smmu_pmu_setup_msi': >> drivers//perf/arm_smmuv3_pmu.c:632:8: error: implicit declaration of >> function 'platform_msi_domain_alloc_irqs'; did you mean >> 'platform_device_alloc'? [-Werror=implicit-function-declaration] ret = platform_msi_domain_alloc_irqs(dev, 1, smmu_pmu_write_msi_msg); ^~ platform_device_alloc In file included from include/linux/list.h:9:0, from include/linux/resource_ext.h:17, from include/linux/acpi.h:26, from drivers//perf/arm_smmuv3_pmu.c:37: include/linux/msi.h:114:38: error: 'struct device' has no member named 'msi_list' #define dev_to_msi_list(dev) (&(dev)->msi_list) ^ include/linux/kernel.h:961:26: note: in definition of macro 'container_of' void *__mptr = (void *)(ptr); \ ^~~ include/linux/list.h:377:2: note: in expansion of macro 'list_entry' list_entry((ptr)->next, type, member) ^~ include/linux/msi.h:116:2: note: in expansion of macro 'list_first_entry' list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list) ^~~~ include/linux/msi.h:116:19: note: in expansion of macro 'dev_to_msi_list' list_first_entry(dev_to_msi_list((dev)), struct msi_desc, list) ^~~ >> drivers//perf/arm_smmuv3_pmu.c:638:9: note: in expansion of macro >> 'first_msi_entry' desc = first_msi_entry(dev); ^~~ In file included from include/linux/ioport.h:13:0, from include/linux/acpi.h:25, from drivers//perf/arm_smmuv3_pmu.c:37: include/linux/msi.h:114:38: error: 'struct device' has no member named 'msi_list' #define dev_to_msi_list(dev) (&(dev)->msi_list) ^ include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert' bool __cond = !(condition);\ ^ include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert' _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) ^~~ include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert' #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) ^~ include/linux/kernel.h:962:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
[PATCH] selftests/ftrace: Strip escape sequences for log file
Strip escape sequences from the stream to the ftracetest
summary log file. Note that all test-case results are
dumped raw in each file.
Signed-off-by: Masami Hiramatsu
---
tools/testing/selftests/ftrace/ftracetest | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/ftrace/ftracetest
b/tools/testing/selftests/ftrace/ftracetest
index d987bbec675f..75244db70331 100755
--- a/tools/testing/selftests/ftrace/ftracetest
+++ b/tools/testing/selftests/ftrace/ftracetest
@@ -167,11 +167,18 @@ if [ -t 1 -a "$ncolors" -a "$ncolors" -ge 8 ]; then
color_blue="\e[34m"
fi
+strip_esc() {
+ # busybox sed implementation doesn't accept "\x1B", so use [:cntrl:] instead.
+ sed -E "s/[[:cntrl:]]\[([0-9]{1,2}(;[0-9]{1,2})?)?[m|K]//g"
+}
+
prlog() { # messages
- [ -z "$LOG_FILE" ] && echo -e "$@" || echo -e "$@" | tee -a $LOG_FILE
+ echo -e "$@"
+ [ "$LOG_FILE" ] && echo -e "$@" | strip_esc >> $LOG_FILE
}
catlog() { #file
- [ -z "$LOG_FILE" ] && cat $1 || cat $1 | tee -a $LOG_FILE
+ cat $1
+ [ "$LOG_FILE" ] && cat $1 | strip_esc >> $LOG_FILE
}
prlog "=== Ftrace unit tests ==="
[PATCH] selftests/ftrace: Strip escape sequences for log file
Strip escape sequences from the stream to the ftracetest
summary log file. Note that all test-case results are
dumped raw in each file.
Signed-off-by: Masami Hiramatsu
---
tools/testing/selftests/ftrace/ftracetest | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/ftrace/ftracetest
b/tools/testing/selftests/ftrace/ftracetest
index d987bbec675f..75244db70331 100755
--- a/tools/testing/selftests/ftrace/ftracetest
+++ b/tools/testing/selftests/ftrace/ftracetest
@@ -167,11 +167,18 @@ if [ -t 1 -a "$ncolors" -a "$ncolors" -ge 8 ]; then
color_blue="\e[34m"
fi
+strip_esc() {
+ # busybox sed implementation doesn't accept "\x1B", so use [:cntrl:] instead.
+ sed -E "s/[[:cntrl:]]\[([0-9]{1,2}(;[0-9]{1,2})?)?[m|K]//g"
+}
+
prlog() { # messages
- [ -z "$LOG_FILE" ] && echo -e "$@" || echo -e "$@" | tee -a $LOG_FILE
+ echo -e "$@"
+ [ "$LOG_FILE" ] && echo -e "$@" | strip_esc >> $LOG_FILE
}
catlog() { #file
- [ -z "$LOG_FILE" ] && cat $1 || cat $1 | tee -a $LOG_FILE
+ cat $1
+ [ "$LOG_FILE" ] && cat $1 | strip_esc >> $LOG_FILE
}
prlog "=== Ftrace unit tests ==="
Re: [PATCH 0/4] Add clock support for Hi3670 SoC
On Thu, Sep 20, 2018 at 11:00:59PM -0700, Manivannan Sadhasivam wrote: > This patchset adds clock support for Hi3670 SoC from HiSilicon utilizing > the HiSi common clock code. While adding clock support, let's remove the > fixed clock for UART and source SoC clock on HiKey970 board. > > This patchset has been verified on HiKey970 board. > > Thanks, > Mani > > Manivannan Sadhasivam (4): > dt-bindings: clk: hisilicon: Add bindings for Hi3670 clk > arm64: dts: hisilicon: Add clock nodes for Hi3670 SoC > arm64: dts: hisilicon: Source SoC clock for UART6 Hi Wei, Since the clk patches are merged, can you please take the DTS patches through HiSi tree? I will post the pinctrl/gpio patches soon which will depend on clk bits. Thanks, Mani > clk: hisilicon: Add clock driver for Hi3670 SoC > > .../bindings/clock/hi3670-clock.txt | 43 + > arch/arm64/boot/dts/hisilicon/hi3670.dtsi | 48 +- > drivers/clk/hisilicon/Kconfig |7 + > drivers/clk/hisilicon/Makefile|1 + > drivers/clk/hisilicon/clk-hi3670.c| 1016 + > include/dt-bindings/clock/hi3670-clock.h | 348 ++ > 6 files changed, 1458 insertions(+), 5 deletions(-) > create mode 100644 Documentation/devicetree/bindings/clock/hi3670-clock.txt > create mode 100644 drivers/clk/hisilicon/clk-hi3670.c > create mode 100644 include/dt-bindings/clock/hi3670-clock.h > > -- > 2.17.1 >
Re: [PATCH 0/4] Add clock support for Hi3670 SoC
On Thu, Sep 20, 2018 at 11:00:59PM -0700, Manivannan Sadhasivam wrote: > This patchset adds clock support for Hi3670 SoC from HiSilicon utilizing > the HiSi common clock code. While adding clock support, let's remove the > fixed clock for UART and source SoC clock on HiKey970 board. > > This patchset has been verified on HiKey970 board. > > Thanks, > Mani > > Manivannan Sadhasivam (4): > dt-bindings: clk: hisilicon: Add bindings for Hi3670 clk > arm64: dts: hisilicon: Add clock nodes for Hi3670 SoC > arm64: dts: hisilicon: Source SoC clock for UART6 Hi Wei, Since the clk patches are merged, can you please take the DTS patches through HiSi tree? I will post the pinctrl/gpio patches soon which will depend on clk bits. Thanks, Mani > clk: hisilicon: Add clock driver for Hi3670 SoC > > .../bindings/clock/hi3670-clock.txt | 43 + > arch/arm64/boot/dts/hisilicon/hi3670.dtsi | 48 +- > drivers/clk/hisilicon/Kconfig |7 + > drivers/clk/hisilicon/Makefile|1 + > drivers/clk/hisilicon/clk-hi3670.c| 1016 + > include/dt-bindings/clock/hi3670-clock.h | 348 ++ > 6 files changed, 1458 insertions(+), 5 deletions(-) > create mode 100644 Documentation/devicetree/bindings/clock/hi3670-clock.txt > create mode 100644 drivers/clk/hisilicon/clk-hi3670.c > create mode 100644 include/dt-bindings/clock/hi3670-clock.h > > -- > 2.17.1 >
Re: [PATCH 11/19] PCI: keystone: Cleanup PHY handling
Lorenzo,
On Tuesday 16 October 2018 10:36 PM, Lorenzo Pieralisi wrote:
> On Mon, Oct 15, 2018 at 06:37:13PM +0530, Kishon Vijay Abraham I wrote:
>> Cleanup PHY handling by using devm_phy_optional_get to get PHYs if
>> the PHYs are optional, creating a device link between the PHY device
>> and the controller device and disable PHY on error cases here.
>> Also invoke phy_reset() as part of initializing PHY.
>
> Hi Kishon,
>
> it is a bit of nitpicking, I know it is annoying but when I read "Also"
> in commit logs there is almost certainly a reason to split the patch
> into logical standalone entities, this one looks like one.
>
> Every patch must be a self-contained change that, in case we have to
> revert it, must not affect other patches.
>
> There are some more patches in the series that would benefit from
> splitting so I kindly ask you to go through them and repost, we
> should still be able to hit v4.20.
Sure, I'll repost them.
Thanks
Kishon
>
> Thanks,
> Lorenzo
>
>> Signed-off-by: Kishon Vijay Abraham I
>> ---
>> drivers/pci/controller/dwc/pci-keystone.c | 126 +++---
>> 1 file changed, 110 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pci-keystone.c
>> b/drivers/pci/controller/dwc/pci-keystone.c
>> index e22328f89c84..bf37609ad75b 100644
>> --- a/drivers/pci/controller/dwc/pci-keystone.c
>> +++ b/drivers/pci/controller/dwc/pci-keystone.c
>> @@ -105,6 +105,9 @@ struct keystone_pcie {
>>
>> int num_msi_host_irqs;
>> int msi_host_irqs[MAX_MSI_HOST_IRQS];
>> +int num_lanes;
>> +struct phy **phy;
>> +struct device_link **link;
>> struct device_node *msi_intc_np;
>> struct irq_domain *legacy_irq_domain;
>> struct device_node *np;
>> @@ -880,22 +883,61 @@ static const struct dw_pcie_ops ks_pcie_dw_pcie_ops = {
>> .link_up = ks_pcie_link_up,
>> };
>>
>> -static int __exit ks_pcie_remove(struct platform_device *pdev)
>> +static void ks_pcie_disable_phy(struct keystone_pcie *ks_pcie)
>> {
>> -struct keystone_pcie *ks_pcie = platform_get_drvdata(pdev);
>> +int num_lanes = ks_pcie->num_lanes;
>>
>> -clk_disable_unprepare(ks_pcie->clk);
>> +while (num_lanes--) {
>> +phy_power_off(ks_pcie->phy[num_lanes]);
>> +phy_exit(ks_pcie->phy[num_lanes]);
>> +}
>> +}
>> +
>> +static int ks_pcie_enable_phy(struct keystone_pcie *ks_pcie)
>> +{
>> +int i;
>> +int ret;
>> +int num_lanes = ks_pcie->num_lanes;
>> +
>> +for (i = 0; i < num_lanes; i++) {
>> +ret = phy_reset(ks_pcie->phy[i]);
>> +if (ret < 0)
>> +goto err_phy;
>> +
>> +ret = phy_init(ks_pcie->phy[i]);
>> +if (ret < 0)
>> +goto err_phy;
>> +
>> +ret = phy_power_on(ks_pcie->phy[i]);
>> +if (ret < 0) {
>> +phy_exit(ks_pcie->phy[i]);
>> +goto err_phy;
>> +}
>> +}
>>
>> return 0;
>> +
>> +err_phy:
>> +while (--i >= 0) {
>> +phy_power_off(ks_pcie->phy[i]);
>> +phy_exit(ks_pcie->phy[i]);
>> +}
>> +
>> +return ret;
>> }
>>
>> static int __init ks_pcie_probe(struct platform_device *pdev)
>> {
>> struct device *dev = >dev;
>> +struct device_node *np = dev->of_node;
>> struct dw_pcie *pci;
>> struct keystone_pcie *ks_pcie;
>> -struct phy *phy;
>> +struct device_link **link;
>> +struct phy **phy;
>> +u32 num_lanes;
>> +char name[10];
>> int ret;
>> +int i;
>>
>> ks_pcie = devm_kzalloc(dev, sizeof(*ks_pcie), GFP_KERNEL);
>> if (!ks_pcie)
>> @@ -908,29 +950,59 @@ static int __init ks_pcie_probe(struct platform_device
>> *pdev)
>> pci->dev = dev;
>> pci->ops = _pcie_dw_pcie_ops;
>>
>> -ks_pcie->pci = pci;
>> +ret = of_property_read_u32(np, "num-lanes", _lanes);
>> +if (ret)
>> +num_lanes = 1;
>>
>> -/* initialize SerDes Phy if present */
>> -phy = devm_phy_get(dev, "pcie-phy");
>> -if (PTR_ERR_OR_ZERO(phy) == -EPROBE_DEFER)
>> -return PTR_ERR(phy);
>> +phy = devm_kzalloc(dev, sizeof(*phy) * num_lanes, GFP_KERNEL);
>> +if (!phy)
>> +return -ENOMEM;
>>
>> -if (!IS_ERR_OR_NULL(phy)) {
>> -ret = phy_init(phy);
>> -if (ret < 0)
>> -return ret;
>> +link = devm_kzalloc(dev, sizeof(*link) * num_lanes, GFP_KERNEL);
>> +if (!link)
>> +return -ENOMEM;
>> +
>> +for (i = 0; i < num_lanes; i++) {
>> +snprintf(name, sizeof(name), "pcie-phy%d", i);
>> +phy[i] = devm_phy_optional_get(dev, name);
>> +if (IS_ERR(phy[i])) {
>> +ret = PTR_ERR(phy[i]);
>> +goto err_link;
>> +}
>> +
>> +if (!phy[i])
>> +
Re: [PATCH 11/19] PCI: keystone: Cleanup PHY handling
Lorenzo,
On Tuesday 16 October 2018 10:36 PM, Lorenzo Pieralisi wrote:
> On Mon, Oct 15, 2018 at 06:37:13PM +0530, Kishon Vijay Abraham I wrote:
>> Cleanup PHY handling by using devm_phy_optional_get to get PHYs if
>> the PHYs are optional, creating a device link between the PHY device
>> and the controller device and disable PHY on error cases here.
>> Also invoke phy_reset() as part of initializing PHY.
>
> Hi Kishon,
>
> it is a bit of nitpicking, I know it is annoying but when I read "Also"
> in commit logs there is almost certainly a reason to split the patch
> into logical standalone entities, this one looks like one.
>
> Every patch must be a self-contained change that, in case we have to
> revert it, must not affect other patches.
>
> There are some more patches in the series that would benefit from
> splitting so I kindly ask you to go through them and repost, we
> should still be able to hit v4.20.
Sure, I'll repost them.
Thanks
Kishon
>
> Thanks,
> Lorenzo
>
>> Signed-off-by: Kishon Vijay Abraham I
>> ---
>> drivers/pci/controller/dwc/pci-keystone.c | 126 +++---
>> 1 file changed, 110 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pci-keystone.c
>> b/drivers/pci/controller/dwc/pci-keystone.c
>> index e22328f89c84..bf37609ad75b 100644
>> --- a/drivers/pci/controller/dwc/pci-keystone.c
>> +++ b/drivers/pci/controller/dwc/pci-keystone.c
>> @@ -105,6 +105,9 @@ struct keystone_pcie {
>>
>> int num_msi_host_irqs;
>> int msi_host_irqs[MAX_MSI_HOST_IRQS];
>> +int num_lanes;
>> +struct phy **phy;
>> +struct device_link **link;
>> struct device_node *msi_intc_np;
>> struct irq_domain *legacy_irq_domain;
>> struct device_node *np;
>> @@ -880,22 +883,61 @@ static const struct dw_pcie_ops ks_pcie_dw_pcie_ops = {
>> .link_up = ks_pcie_link_up,
>> };
>>
>> -static int __exit ks_pcie_remove(struct platform_device *pdev)
>> +static void ks_pcie_disable_phy(struct keystone_pcie *ks_pcie)
>> {
>> -struct keystone_pcie *ks_pcie = platform_get_drvdata(pdev);
>> +int num_lanes = ks_pcie->num_lanes;
>>
>> -clk_disable_unprepare(ks_pcie->clk);
>> +while (num_lanes--) {
>> +phy_power_off(ks_pcie->phy[num_lanes]);
>> +phy_exit(ks_pcie->phy[num_lanes]);
>> +}
>> +}
>> +
>> +static int ks_pcie_enable_phy(struct keystone_pcie *ks_pcie)
>> +{
>> +int i;
>> +int ret;
>> +int num_lanes = ks_pcie->num_lanes;
>> +
>> +for (i = 0; i < num_lanes; i++) {
>> +ret = phy_reset(ks_pcie->phy[i]);
>> +if (ret < 0)
>> +goto err_phy;
>> +
>> +ret = phy_init(ks_pcie->phy[i]);
>> +if (ret < 0)
>> +goto err_phy;
>> +
>> +ret = phy_power_on(ks_pcie->phy[i]);
>> +if (ret < 0) {
>> +phy_exit(ks_pcie->phy[i]);
>> +goto err_phy;
>> +}
>> +}
>>
>> return 0;
>> +
>> +err_phy:
>> +while (--i >= 0) {
>> +phy_power_off(ks_pcie->phy[i]);
>> +phy_exit(ks_pcie->phy[i]);
>> +}
>> +
>> +return ret;
>> }
>>
>> static int __init ks_pcie_probe(struct platform_device *pdev)
>> {
>> struct device *dev = >dev;
>> +struct device_node *np = dev->of_node;
>> struct dw_pcie *pci;
>> struct keystone_pcie *ks_pcie;
>> -struct phy *phy;
>> +struct device_link **link;
>> +struct phy **phy;
>> +u32 num_lanes;
>> +char name[10];
>> int ret;
>> +int i;
>>
>> ks_pcie = devm_kzalloc(dev, sizeof(*ks_pcie), GFP_KERNEL);
>> if (!ks_pcie)
>> @@ -908,29 +950,59 @@ static int __init ks_pcie_probe(struct platform_device
>> *pdev)
>> pci->dev = dev;
>> pci->ops = _pcie_dw_pcie_ops;
>>
>> -ks_pcie->pci = pci;
>> +ret = of_property_read_u32(np, "num-lanes", _lanes);
>> +if (ret)
>> +num_lanes = 1;
>>
>> -/* initialize SerDes Phy if present */
>> -phy = devm_phy_get(dev, "pcie-phy");
>> -if (PTR_ERR_OR_ZERO(phy) == -EPROBE_DEFER)
>> -return PTR_ERR(phy);
>> +phy = devm_kzalloc(dev, sizeof(*phy) * num_lanes, GFP_KERNEL);
>> +if (!phy)
>> +return -ENOMEM;
>>
>> -if (!IS_ERR_OR_NULL(phy)) {
>> -ret = phy_init(phy);
>> -if (ret < 0)
>> -return ret;
>> +link = devm_kzalloc(dev, sizeof(*link) * num_lanes, GFP_KERNEL);
>> +if (!link)
>> +return -ENOMEM;
>> +
>> +for (i = 0; i < num_lanes; i++) {
>> +snprintf(name, sizeof(name), "pcie-phy%d", i);
>> +phy[i] = devm_phy_optional_get(dev, name);
>> +if (IS_ERR(phy[i])) {
>> +ret = PTR_ERR(phy[i]);
>> +goto err_link;
>> +}
>> +
>> +if (!phy[i])
>> +
[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
[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
[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
[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
[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
[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
[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
[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
[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
[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
[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
[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
Re: [Lkcamp] [PATCH 3/4] kbuild: Removes unnecessary shadowed local variable and optimize testing.
Hi Leonardo,
On 10/16/18 9:09 PM, Leonardo Brás wrote:
> Removes an unnecessary shadowed local variable (start).
> Optimize test of isdigit:
> - If isalpha returns true, isdigit will return false, so no need to test.
This patch does two different things, it should be in two separated patches.
>
> Signed-off-by: Leonardo Brás
> ---
> scripts/asn1_compiler.c | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/asn1_compiler.c b/scripts/asn1_compiler.c
> index c146020fc783..08bb6e5fd6ad 100644
> --- a/scripts/asn1_compiler.c
> +++ b/scripts/asn1_compiler.c
> @@ -413,7 +413,7 @@ static void tokenise(char *buffer, char *end)
>
> /* Handle string tokens */
> if (isalpha(*p)) {
> - const char **dir, *start = p;
> + const char **dir;
>
> /* Can be a directive, type name or element
>* name. Find the end of the name.
> @@ -454,10 +454,9 @@ static void tokenise(char *buffer, char *end)
>
> tokens[tix++].token_type = TOKEN_TYPE_NAME;
> continue;
> - }
> + } else if (isdigit(*p)) {
> + /* Handle numbers */
Actually you can't do that, p is being altered in the first if statement.
>
> - /* Handle numbers */
> - if (isdigit(*p)) {
> /* Find the end of the number */
> q = p + 1;
> while (q < nl && (isdigit(*q)))
>
Regards
Helen
Re: [Lkcamp] [PATCH 3/4] kbuild: Removes unnecessary shadowed local variable and optimize testing.
Hi Leonardo,
On 10/16/18 9:09 PM, Leonardo Brás wrote:
> Removes an unnecessary shadowed local variable (start).
> Optimize test of isdigit:
> - If isalpha returns true, isdigit will return false, so no need to test.
This patch does two different things, it should be in two separated patches.
>
> Signed-off-by: Leonardo Brás
> ---
> scripts/asn1_compiler.c | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/asn1_compiler.c b/scripts/asn1_compiler.c
> index c146020fc783..08bb6e5fd6ad 100644
> --- a/scripts/asn1_compiler.c
> +++ b/scripts/asn1_compiler.c
> @@ -413,7 +413,7 @@ static void tokenise(char *buffer, char *end)
>
> /* Handle string tokens */
> if (isalpha(*p)) {
> - const char **dir, *start = p;
> + const char **dir;
>
> /* Can be a directive, type name or element
>* name. Find the end of the name.
> @@ -454,10 +454,9 @@ static void tokenise(char *buffer, char *end)
>
> tokens[tix++].token_type = TOKEN_TYPE_NAME;
> continue;
> - }
> + } else if (isdigit(*p)) {
> + /* Handle numbers */
Actually you can't do that, p is being altered in the first if statement.
>
> - /* Handle numbers */
> - if (isdigit(*p)) {
> /* Find the end of the number */
> q = p + 1;
> while (q < nl && (isdigit(*q)))
>
Regards
Helen
Re: [PATCH v3 6/9] arm64: dts: actions: Add Reset Controller support for S900 SoC
On Tue, Oct 16, 2018 at 02:42:19PM -0700, Stephen Boyd wrote:
> Quoting Manivannan Sadhasivam (2018-08-10 02:51:10)
> > Add reset controller property and bindings header for the
> > Actions Semi S900 SoC DTS.
> >
> > Signed-off-by: Manivannan Sadhasivam
> > ---
>
> Are these DT patches necessary for me to apply the clk patches? I'd like
> to apply the clk patches without these dts bits.
>
Not necessary, you can just apply the clk and bindings patches. DTS patches
will go through Actions tree.
Thanks,
Mani
> > arch/arm64/boot/dts/actions/s900.dtsi | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/actions/s900.dtsi
> > b/arch/arm64/boot/dts/actions/s900.dtsi
> > index aa3a49b0d646..4fbb39fd7971 100644
> > --- a/arch/arm64/boot/dts/actions/s900.dtsi
> > +++ b/arch/arm64/boot/dts/actions/s900.dtsi
> > @@ -6,6 +6,7 @@
> >
> > #include
> > #include
> > +#include
> >
> > / {
> > compatible = "actions,s900";
> > @@ -172,6 +173,7 @@
> > reg = <0x0 0xe016 0x0 0x1000>;
> > clocks = <>, <>;
> > #clock-cells = <1>;
> > + #reset-cells = <1>;
> > };
> >
> > pinctrl: pinctrl@e01b {
> > --
> > 2.17.1
> >
Re: [PATCH v3 6/9] arm64: dts: actions: Add Reset Controller support for S900 SoC
On Tue, Oct 16, 2018 at 02:42:19PM -0700, Stephen Boyd wrote:
> Quoting Manivannan Sadhasivam (2018-08-10 02:51:10)
> > Add reset controller property and bindings header for the
> > Actions Semi S900 SoC DTS.
> >
> > Signed-off-by: Manivannan Sadhasivam
> > ---
>
> Are these DT patches necessary for me to apply the clk patches? I'd like
> to apply the clk patches without these dts bits.
>
Not necessary, you can just apply the clk and bindings patches. DTS patches
will go through Actions tree.
Thanks,
Mani
> > arch/arm64/boot/dts/actions/s900.dtsi | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/actions/s900.dtsi
> > b/arch/arm64/boot/dts/actions/s900.dtsi
> > index aa3a49b0d646..4fbb39fd7971 100644
> > --- a/arch/arm64/boot/dts/actions/s900.dtsi
> > +++ b/arch/arm64/boot/dts/actions/s900.dtsi
> > @@ -6,6 +6,7 @@
> >
> > #include
> > #include
> > +#include
> >
> > / {
> > compatible = "actions,s900";
> > @@ -172,6 +173,7 @@
> > reg = <0x0 0xe016 0x0 0x1000>;
> > clocks = <>, <>;
> > #clock-cells = <1>;
> > + #reset-cells = <1>;
> > };
> >
> > pinctrl: pinctrl@e01b {
> > --
> > 2.17.1
> >
Re: [Lkcamp] [PATCH 2/4] Renames variable to fix shadow warning.
Hi Leonardo,
Thanks for the patch, just some small comments below.
Please, check previous log messages with git log
arch/x86/entry/vdso/vdso2c.h, you will see that most patches had the
prefix "x86/vdso:" in the commit message.
On 10/16/18 9:08 PM, Leonardo Brás wrote:
> Renames the char variable to avoid shadowing a variable previously
> declared on this function.
>
> Signed-off-by: Leonardo Brás
> ---
> arch/x86/entry/vdso/vdso2c.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/entry/vdso/vdso2c.h b/arch/x86/entry/vdso/vdso2c.h
> index fa847a620f40..9466998d0f28 100644
> --- a/arch/x86/entry/vdso/vdso2c.h
> +++ b/arch/x86/entry/vdso/vdso2c.h
> @@ -93,11 +93,11 @@ static void BITSFUNC(go)(void *raw_addr, size_t raw_len,
> int k;
> ELF(Sym) *sym = raw_addr + GET_LE(_hdr->sh_offset) +
> GET_LE(_hdr->sh_entsize) * i;
> - const char *name = raw_addr + GET_LE(_hdr->sh_offset) +
> + const char *name2 = raw_addr + GET_LE(_hdr->sh_offset) +
> GET_LE(>st_name);
Could you please use a more meaningful name instead of name2 please? I
believe sym_name should be fine.
>
> for (k = 0; k < NSYMS; k++) {
> - if (!strcmp(name, required_syms[k].name)) {
> + if (!strcmp(name2, required_syms[k].name)) {
> if (syms[k]) {
> fail("duplicate symbol %s\n",
>required_syms[k].name);
>
Regards
Helen
Re: [Lkcamp] [PATCH 2/4] Renames variable to fix shadow warning.
Hi Leonardo,
Thanks for the patch, just some small comments below.
Please, check previous log messages with git log
arch/x86/entry/vdso/vdso2c.h, you will see that most patches had the
prefix "x86/vdso:" in the commit message.
On 10/16/18 9:08 PM, Leonardo Brás wrote:
> Renames the char variable to avoid shadowing a variable previously
> declared on this function.
>
> Signed-off-by: Leonardo Brás
> ---
> arch/x86/entry/vdso/vdso2c.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/entry/vdso/vdso2c.h b/arch/x86/entry/vdso/vdso2c.h
> index fa847a620f40..9466998d0f28 100644
> --- a/arch/x86/entry/vdso/vdso2c.h
> +++ b/arch/x86/entry/vdso/vdso2c.h
> @@ -93,11 +93,11 @@ static void BITSFUNC(go)(void *raw_addr, size_t raw_len,
> int k;
> ELF(Sym) *sym = raw_addr + GET_LE(_hdr->sh_offset) +
> GET_LE(_hdr->sh_entsize) * i;
> - const char *name = raw_addr + GET_LE(_hdr->sh_offset) +
> + const char *name2 = raw_addr + GET_LE(_hdr->sh_offset) +
> GET_LE(>st_name);
Could you please use a more meaningful name instead of name2 please? I
believe sym_name should be fine.
>
> for (k = 0; k < NSYMS; k++) {
> - if (!strcmp(name, required_syms[k].name)) {
> + if (!strcmp(name2, required_syms[k].name)) {
> if (syms[k]) {
> fail("duplicate symbol %s\n",
>required_syms[k].name);
>
Regards
Helen
Re: [PATCH v9 5/5] lib/dlock-list: Scale dlock_lists_empty()
On Thu, 04 Oct 2018, Waiman Long wrote: On 10/04/2018 03:16 AM, Jan Kara wrote: On Wed 12-09-18 15:28:52, Waiman Long wrote: From: Davidlohr Bueso Instead of the current O(N) implementation, at the cost of adding an atomic counter, we can convert the call to an atomic_read(). The counter only serves for accounting empty to non-empty transitions, and vice versa; therefore only modified twice for each of the lists during the lifetime of the dlock (while used). In addition, to be able to unaccount a list_del(), we add a dlist pointer to each head, thus minimizing the overall memory footprint. Signed-off-by: Davidlohr Bueso Acked-by: Waiman Long So I was wondering: Is this really worth it? AFAICS we have a single call site for dlock_lists_empty() and that happens during umount where we don't really care about this optimization. So it seems like unnecessary complication to me at this point? If someone comes up with a usecase that needs fast dlock_lists_empty(), then sure, we can do this... Yes, that is true. We can skip this patch for the time being until a use case comes up which requires dlock_lists_empty() to be used in the fast path. So fyi I ended up porting the epoll ready-list to this api, where dlock_lists_empty() performance _does_ matter. However, the list iteration is common enough operation to put perform the benefits of the percpu add/delete operations. For example, when sending ready events to userspace (ep_send_events_proc()), each item must drop the iter lock, and also do a delete operation -- similarly for checking for ready events (ep_read_events_proc). This ends hurting more than benefiting workloads. Anyway, so yeah I have no need for this patch, and the added complexity + atomics is unjustified. Thanks, Davidlohr
Re: [PATCH v9 5/5] lib/dlock-list: Scale dlock_lists_empty()
On Thu, 04 Oct 2018, Waiman Long wrote: On 10/04/2018 03:16 AM, Jan Kara wrote: On Wed 12-09-18 15:28:52, Waiman Long wrote: From: Davidlohr Bueso Instead of the current O(N) implementation, at the cost of adding an atomic counter, we can convert the call to an atomic_read(). The counter only serves for accounting empty to non-empty transitions, and vice versa; therefore only modified twice for each of the lists during the lifetime of the dlock (while used). In addition, to be able to unaccount a list_del(), we add a dlist pointer to each head, thus minimizing the overall memory footprint. Signed-off-by: Davidlohr Bueso Acked-by: Waiman Long So I was wondering: Is this really worth it? AFAICS we have a single call site for dlock_lists_empty() and that happens during umount where we don't really care about this optimization. So it seems like unnecessary complication to me at this point? If someone comes up with a usecase that needs fast dlock_lists_empty(), then sure, we can do this... Yes, that is true. We can skip this patch for the time being until a use case comes up which requires dlock_lists_empty() to be used in the fast path. So fyi I ended up porting the epoll ready-list to this api, where dlock_lists_empty() performance _does_ matter. However, the list iteration is common enough operation to put perform the benefits of the percpu add/delete operations. For example, when sending ready events to userspace (ep_send_events_proc()), each item must drop the iter lock, and also do a delete operation -- similarly for checking for ready events (ep_read_events_proc). This ends hurting more than benefiting workloads. Anyway, so yeah I have no need for this patch, and the added complexity + atomics is unjustified. Thanks, Davidlohr
Re: [Ksummit-discuss] [PATCH v3 1/3] code-of-conduct: Fix the ambiguity about collecting email addresses
On Tue, 2018-10-16 at 19:10 -0700, Frank Rowand wrote:
> On 10/16/18 07:58, James Bottomley wrote:
> > The current code of conduct has an ambiguity in the it considers
> > publishing
> > private information such as email addresses unacceptable
> > behaviour. Since
> > the Linux kernel collects and publishes email addresses as part of
> > the patch
> > process, add an exception clause for email addresses ordinarily
> > collected by
> > the project to correct this ambiguity.
> >
> > Fixes: 8a104f8b5867c682 ("Code of Conduct: Let's revamp it.")
> > Acked-by: Geert Uytterhoeven
> > Acked-by: Shuah Khan
> > Acked-by: Guenter Roeck
> > Reviewed-by: Alan Cox
> > Reviewed-by: Mauro Carvalho Chehab
> > Acked-by: "Eric W. Biederman"
> > Acked-by: Kees Cook
> > Signed-off-by: James Bottomley > om>
> > ---
> > Documentation/process/code-of-conduct.rst | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Documentation/process/code-of-conduct.rst
> > b/Documentation/process/code-of-conduct.rst
> > index ab7c24b5478c..aa40e34e7785 100644
> > --- a/Documentation/process/code-of-conduct.rst
> > +++ b/Documentation/process/code-of-conduct.rst
> > @@ -31,7 +31,7 @@ Examples of unacceptable behavior by participants
> > include:
> > * Trolling, insulting/derogatory comments, and personal or
> > political attacks
> > * Public or private harassment
> > * Publishing others’ private information, such as a physical or
> > electronic
> > - address, without explicit permission
> > + address not ordinarily collected by the project, without
> > explicit permission
> > * Other conduct which could reasonably be considered inappropriate
> > in a
> >professional setting
> >
> >
>
> Repeating my comment on version 1:
>
> My understanding of the concern behind this change is that we should
> be able to use an email address for the current development
> practices, such as Reported-by, Suggested-by, etc tags when the email
> address was provided in what is a public space for the project. The
> public space is visible to anyone in the world who desires to access
> it.
>
> I do not understand how "ordinarily collected by the project" is
> equivalent to "an email address that was provided in a public space
> for the project".
I don't think it is ... or should be. This section is specifically
enumerating unacceptable behaviours. The carve out "email address not
ordinarily collected by the project" means that adding someone's email
address in a tag isn't immediately sanctionable in the code of conduct
as unacceptable behaviour if a question about whether you asked
explicit permission arises. Equally, a carve out from unacceptable
behaviours doesn't make the action always acceptable, so it's not a
licence to publish someone's email address regardless of context.
> Ordinarily collected could include activities that can be expected to
> be private and not visible to any arbitrary person in the world.
It's not a blanket permission, it's an exclusion from being considered
unacceptable behaviour. I would be interested to know what information
we ordinarily collect in the course of building linux that should be
considered private because I might have missed something about the
implications here.
James
> My issue is with the word choice. I agree with the underlying
> concept.
>
> -Frank
> ___
> Ksummit-discuss mailing list
> ksummit-disc...@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss
Re: [Ksummit-discuss] [PATCH v3 1/3] code-of-conduct: Fix the ambiguity about collecting email addresses
On Tue, 2018-10-16 at 19:10 -0700, Frank Rowand wrote:
> On 10/16/18 07:58, James Bottomley wrote:
> > The current code of conduct has an ambiguity in the it considers
> > publishing
> > private information such as email addresses unacceptable
> > behaviour. Since
> > the Linux kernel collects and publishes email addresses as part of
> > the patch
> > process, add an exception clause for email addresses ordinarily
> > collected by
> > the project to correct this ambiguity.
> >
> > Fixes: 8a104f8b5867c682 ("Code of Conduct: Let's revamp it.")
> > Acked-by: Geert Uytterhoeven
> > Acked-by: Shuah Khan
> > Acked-by: Guenter Roeck
> > Reviewed-by: Alan Cox
> > Reviewed-by: Mauro Carvalho Chehab
> > Acked-by: "Eric W. Biederman"
> > Acked-by: Kees Cook
> > Signed-off-by: James Bottomley > om>
> > ---
> > Documentation/process/code-of-conduct.rst | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Documentation/process/code-of-conduct.rst
> > b/Documentation/process/code-of-conduct.rst
> > index ab7c24b5478c..aa40e34e7785 100644
> > --- a/Documentation/process/code-of-conduct.rst
> > +++ b/Documentation/process/code-of-conduct.rst
> > @@ -31,7 +31,7 @@ Examples of unacceptable behavior by participants
> > include:
> > * Trolling, insulting/derogatory comments, and personal or
> > political attacks
> > * Public or private harassment
> > * Publishing others’ private information, such as a physical or
> > electronic
> > - address, without explicit permission
> > + address not ordinarily collected by the project, without
> > explicit permission
> > * Other conduct which could reasonably be considered inappropriate
> > in a
> >professional setting
> >
> >
>
> Repeating my comment on version 1:
>
> My understanding of the concern behind this change is that we should
> be able to use an email address for the current development
> practices, such as Reported-by, Suggested-by, etc tags when the email
> address was provided in what is a public space for the project. The
> public space is visible to anyone in the world who desires to access
> it.
>
> I do not understand how "ordinarily collected by the project" is
> equivalent to "an email address that was provided in a public space
> for the project".
I don't think it is ... or should be. This section is specifically
enumerating unacceptable behaviours. The carve out "email address not
ordinarily collected by the project" means that adding someone's email
address in a tag isn't immediately sanctionable in the code of conduct
as unacceptable behaviour if a question about whether you asked
explicit permission arises. Equally, a carve out from unacceptable
behaviours doesn't make the action always acceptable, so it's not a
licence to publish someone's email address regardless of context.
> Ordinarily collected could include activities that can be expected to
> be private and not visible to any arbitrary person in the world.
It's not a blanket permission, it's an exclusion from being considered
unacceptable behaviour. I would be interested to know what information
we ordinarily collect in the course of building linux that should be
considered private because I might have missed something about the
implications here.
James
> My issue is with the word choice. I agree with the underlying
> concept.
>
> -Frank
> ___
> Ksummit-discuss mailing list
> ksummit-disc...@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss
[PATCH] scripts/gdb: fix lx-version for gdb 7.3-
For gdb version less than 7.3, lx-version only one character.
(gdb) lx-version
L(gdb)
This can be fixed by casting 'linux_banner' as (char *).
(gdb) lx-version
Linux version 4.19.0-rc1+ (changbin@acer) (gcc version 7.3.0 (Ubuntu
7.3.0-16ubuntu3)) #21 SMP Sat Sep 1 21:43:30 CST 2018
gdb 7.4 seems to be no such issue.
Signed-off-by: Du Changbin
---
scripts/gdb/linux/proc.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/gdb/linux/proc.py b/scripts/gdb/linux/proc.py
index 086d27223c0c..0aebd7565b03 100644
--- a/scripts/gdb/linux/proc.py
+++ b/scripts/gdb/linux/proc.py
@@ -41,7 +41,7 @@ class LxVersion(gdb.Command):
def invoke(self, arg, from_tty):
# linux_banner should contain a newline
-gdb.write(gdb.parse_and_eval("linux_banner").string())
+gdb.write(gdb.parse_and_eval("(char *)linux_banner").string())
LxVersion()
--
2.17.1
[PATCH] scripts/gdb: fix lx-version for gdb 7.3-
For gdb version less than 7.3, lx-version only one character.
(gdb) lx-version
L(gdb)
This can be fixed by casting 'linux_banner' as (char *).
(gdb) lx-version
Linux version 4.19.0-rc1+ (changbin@acer) (gcc version 7.3.0 (Ubuntu
7.3.0-16ubuntu3)) #21 SMP Sat Sep 1 21:43:30 CST 2018
gdb 7.4 seems to be no such issue.
Signed-off-by: Du Changbin
---
scripts/gdb/linux/proc.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/gdb/linux/proc.py b/scripts/gdb/linux/proc.py
index 086d27223c0c..0aebd7565b03 100644
--- a/scripts/gdb/linux/proc.py
+++ b/scripts/gdb/linux/proc.py
@@ -41,7 +41,7 @@ class LxVersion(gdb.Command):
def invoke(self, arg, from_tty):
# linux_banner should contain a newline
-gdb.write(gdb.parse_and_eval("linux_banner").string())
+gdb.write(gdb.parse_and_eval("(char *)linux_banner").string())
LxVersion()
--
2.17.1
Re: [PATCH] mmc: sdhci: use goto rather than return directly
On Tue, 16 Oct 2018 at 16:30, Adrian Hunter wrote:
>
> On 16/10/18 11:19 AM, Chunyan Zhang wrote:
> > The driver should clean resources requested in the function before
> > unnormal return.
> >
> > CC: Linus Walleij
> > Signed-off-by: Chunyan Zhang
> > Fixes: bd9b902798ab ("mmc: sdhci: Implement an SDHCI-specific bounce
> > buffer")
> > ---
> > drivers/mmc/host/sdhci.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > index 1b3fbd9..f6b57e1 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -3991,7 +3991,7 @@ int sdhci_setup_host(struct sdhci_host *host)
> > /* This may alter mmc->*_blk_* parameters */
> > ret = sdhci_allocate_bounce_buffer(host);
>
> Do we need the error return? It seems to be zero always.
Or we don't need to check 'ret' for now, right?
>
> > if (ret)
> > - return ret;
> > + goto unreg;
> > }
> >
> > return 0;
> >
>
Re: [PATCH] mmc: sdhci: use goto rather than return directly
On Tue, 16 Oct 2018 at 16:30, Adrian Hunter wrote:
>
> On 16/10/18 11:19 AM, Chunyan Zhang wrote:
> > The driver should clean resources requested in the function before
> > unnormal return.
> >
> > CC: Linus Walleij
> > Signed-off-by: Chunyan Zhang
> > Fixes: bd9b902798ab ("mmc: sdhci: Implement an SDHCI-specific bounce
> > buffer")
> > ---
> > drivers/mmc/host/sdhci.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > index 1b3fbd9..f6b57e1 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -3991,7 +3991,7 @@ int sdhci_setup_host(struct sdhci_host *host)
> > /* This may alter mmc->*_blk_* parameters */
> > ret = sdhci_allocate_bounce_buffer(host);
>
> Do we need the error return? It seems to be zero always.
Or we don't need to check 'ret' for now, right?
>
> > if (ret)
> > - return ret;
> > + goto unreg;
> > }
> >
> > return 0;
> >
>
Re: [PATCH v3 1/2] sysctl: handle overflow in proc_get_long
On Tue, Oct 16, 2018 at 5:24 PM, Christian Brauner wrote: > Right, but if you write a value that exceeds the buffer of 22 chars that is > used > to parse you already get EINVAL back on current kernels. > So it didn't feel like returning EOVERFLOW or ERANGE might make sense. > I saw a change in 4.10 or 4.11 as well that used EINVAL on UINT_MAX overflow > or something. EINVAL might be enough information for userspace here ?/. I'd agree: I think there is more precedent for EINVAL. -Kees -- Kees Cook Pixel Security
Re: [PATCH v3 1/2] sysctl: handle overflow in proc_get_long
On Tue, Oct 16, 2018 at 5:24 PM, Christian Brauner wrote: > Right, but if you write a value that exceeds the buffer of 22 chars that is > used > to parse you already get EINVAL back on current kernels. > So it didn't feel like returning EOVERFLOW or ERANGE might make sense. > I saw a change in 4.10 or 4.11 as well that used EINVAL on UINT_MAX overflow > or something. EINVAL might be enough information for userspace here ?/. I'd agree: I think there is more precedent for EINVAL. -Kees -- Kees Cook Pixel Security
Re: [Ksummit-discuss] [PATCH v3 1/3] code-of-conduct: Fix the ambiguity about collecting email addresses
On 10/16/18 07:58, James Bottomley wrote:
> The current code of conduct has an ambiguity in the it considers publishing
> private information such as email addresses unacceptable behaviour. Since
> the Linux kernel collects and publishes email addresses as part of the patch
> process, add an exception clause for email addresses ordinarily collected by
> the project to correct this ambiguity.
>
> Fixes: 8a104f8b5867c682 ("Code of Conduct: Let's revamp it.")
> Acked-by: Geert Uytterhoeven
> Acked-by: Shuah Khan
> Acked-by: Guenter Roeck
> Reviewed-by: Alan Cox
> Reviewed-by: Mauro Carvalho Chehab
> Acked-by: "Eric W. Biederman"
> Acked-by: Kees Cook
> Signed-off-by: James Bottomley
> ---
> Documentation/process/code-of-conduct.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/process/code-of-conduct.rst
> b/Documentation/process/code-of-conduct.rst
> index ab7c24b5478c..aa40e34e7785 100644
> --- a/Documentation/process/code-of-conduct.rst
> +++ b/Documentation/process/code-of-conduct.rst
> @@ -31,7 +31,7 @@ Examples of unacceptable behavior by participants include:
> * Trolling, insulting/derogatory comments, and personal or political attacks
> * Public or private harassment
> * Publishing others’ private information, such as a physical or electronic
> - address, without explicit permission
> + address not ordinarily collected by the project, without explicit
> permission
> * Other conduct which could reasonably be considered inappropriate in a
>professional setting
>
>
Repeating my comment on version 1:
My understanding of the concern behind this change is that we should be
able to use an email address for the current development practices, such
as Reported-by, Suggested-by, etc tags when the email address was
provided in what is a public space for the project. The public space
is visible to anyone in the world who desires to access it.
I do not understand how "ordinarily collected by the project" is equivalent
to "an email address that was provided in a public space for the project".
Ordinarily collected could include activities that can be expected to be
private and not visible to any arbitrary person in the world.
My issue is with the word choice. I agree with the underlying concept.
-Frank
Re: [Ksummit-discuss] [PATCH v3 1/3] code-of-conduct: Fix the ambiguity about collecting email addresses
On 10/16/18 07:58, James Bottomley wrote:
> The current code of conduct has an ambiguity in the it considers publishing
> private information such as email addresses unacceptable behaviour. Since
> the Linux kernel collects and publishes email addresses as part of the patch
> process, add an exception clause for email addresses ordinarily collected by
> the project to correct this ambiguity.
>
> Fixes: 8a104f8b5867c682 ("Code of Conduct: Let's revamp it.")
> Acked-by: Geert Uytterhoeven
> Acked-by: Shuah Khan
> Acked-by: Guenter Roeck
> Reviewed-by: Alan Cox
> Reviewed-by: Mauro Carvalho Chehab
> Acked-by: "Eric W. Biederman"
> Acked-by: Kees Cook
> Signed-off-by: James Bottomley
> ---
> Documentation/process/code-of-conduct.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/process/code-of-conduct.rst
> b/Documentation/process/code-of-conduct.rst
> index ab7c24b5478c..aa40e34e7785 100644
> --- a/Documentation/process/code-of-conduct.rst
> +++ b/Documentation/process/code-of-conduct.rst
> @@ -31,7 +31,7 @@ Examples of unacceptable behavior by participants include:
> * Trolling, insulting/derogatory comments, and personal or political attacks
> * Public or private harassment
> * Publishing others’ private information, such as a physical or electronic
> - address, without explicit permission
> + address not ordinarily collected by the project, without explicit
> permission
> * Other conduct which could reasonably be considered inappropriate in a
>professional setting
>
>
Repeating my comment on version 1:
My understanding of the concern behind this change is that we should be
able to use an email address for the current development practices, such
as Reported-by, Suggested-by, etc tags when the email address was
provided in what is a public space for the project. The public space
is visible to anyone in the world who desires to access it.
I do not understand how "ordinarily collected by the project" is equivalent
to "an email address that was provided in a public space for the project".
Ordinarily collected could include activities that can be expected to be
private and not visible to any arbitrary person in the world.
My issue is with the word choice. I agree with the underlying concept.
-Frank
Re: [PATCH] mm/thp: Correctly differentiate between mapped THP and PMD migration entry
Hello Zi, On Sun, Oct 14, 2018 at 08:53:55PM -0400, Zi Yan wrote: > Hi Andrea, what is the purpose/benefit of making x86’s pmd_present() returns > true > for a THP under splitting? Does it cause problems when ARM64’s pmd_present() > returns false in the same situation? !pmd_present means it's a migration entry or swap entry and doesn't point to RAM. It means if you do pmd_to_page(*pmd) it will return you an undefined result. During splitting the physical page is still very well pointed by the pmd as long as pmd_trans_huge returns true and you hold the pmd_lock. pmd_trans_huge must be true at all times for a transhuge pmd that points to a hugepage, or all VM fast paths won't serialize with the pmd_lock, that is the only reason why, and it's a very good reason because it avoids to take the pmd_lock when walking over non transhuge pmds (i.e. when there are no THP allocated). Now if we've to keep _PAGE_PSE set and return true in pmd_trans_huge at all times, why would you want to make pmd_present return false? How could it help if pmd_trans_huge returns true, but pmd_present returns false despite pmd_to_page works fine and the pmd is really still pointing to the page? When userland faults on such pmd !pmd_present it will make the page fault take a swap or migration path, but that's the wrong path if the pmd points to RAM. What we need to do during split is an invalidate of the huge TLB. There's no pmd_trans_splitting anymore, so we only clear the present bit in the PTE despite pmd_present still returns true (just like PROT_NONE, nothing new in this respect). pmd_present never meant the real present bit in the pte was set, it just means the pmd points to RAM. It means it doesn't point to swap or migration entry and you can do pmd_to_page and it works fine. We need to invalidate the TLB by clearing the present bit and by flushing the TLB before overwriting the transhuge pmd with the regular pte (i.e. to make it non huge). That is actually required by an errata (l1 cache aliasing of the same mapping through two different TLB of two different sizes broke some old CPU and triggered machine checks). It's not something fundamentally necessary from a common code point of view. It's more risky from an hardware (not software) standpoint and before you can get rid of the pmd you need to do a TLB flush anyway to be sure CPUs stops using it, so better clear the present bit before doing the real costly thing (the tlb flush with IPIs). Clearing the present bit during the TLB flush is a cost that gets lost in the noise. The clear of the real present bit during pmd (virtual) splitting is done with pmdp_invalidate, that is created specifically to keeps pmd_trans_huge=true, pmd_present=true despite the present bit is not set. So you could imagine _PAGE_PSE as the real present bit. Before the physical split was deferred and decoupled from the virtual memory pmd split, pmd_trans_splitting allowed to wait the split to finish and to keep all gup_fast at bay during it (while the page was still mapped readable and writable in userland by other CPUs). Now the physical split is deferred so you just split the pmd locally and only a physical split invoked on the page (not the virtual split invoked on the pmd with split_huge_pmd) has to keep gup at bay, and it does so by freezing the refcount so all gup_fast fail with the page_cache_get_speculative during the freeze. This removed the need of the pmd_splitting flag in gup_fast (when pmd_splitting was set gup fast had to go through the non-fast gup), but it means that now a hugepage cannot be physically splitted if it's gup pinned. The main difference is that freezing the refcount can fail, so the code must learn to cope with such failure and defer it. Decoupling the physical and virtual splits introduced the need of tracking the doublemap case with a new PG_double_map flag too. It makes the refcounting of hugepages trivial in comparison (identical to hugetlbfs in fact), but it requires total_mapcount to account for all those huge and non huge mappings. It primarily pays off to add THP to tmpfs where the physical split may have to be deferred for pagecache reasons anyway. Thanks, Andrea
Re: [PATCH] mm/thp: Correctly differentiate between mapped THP and PMD migration entry
Hello Zi, On Sun, Oct 14, 2018 at 08:53:55PM -0400, Zi Yan wrote: > Hi Andrea, what is the purpose/benefit of making x86’s pmd_present() returns > true > for a THP under splitting? Does it cause problems when ARM64’s pmd_present() > returns false in the same situation? !pmd_present means it's a migration entry or swap entry and doesn't point to RAM. It means if you do pmd_to_page(*pmd) it will return you an undefined result. During splitting the physical page is still very well pointed by the pmd as long as pmd_trans_huge returns true and you hold the pmd_lock. pmd_trans_huge must be true at all times for a transhuge pmd that points to a hugepage, or all VM fast paths won't serialize with the pmd_lock, that is the only reason why, and it's a very good reason because it avoids to take the pmd_lock when walking over non transhuge pmds (i.e. when there are no THP allocated). Now if we've to keep _PAGE_PSE set and return true in pmd_trans_huge at all times, why would you want to make pmd_present return false? How could it help if pmd_trans_huge returns true, but pmd_present returns false despite pmd_to_page works fine and the pmd is really still pointing to the page? When userland faults on such pmd !pmd_present it will make the page fault take a swap or migration path, but that's the wrong path if the pmd points to RAM. What we need to do during split is an invalidate of the huge TLB. There's no pmd_trans_splitting anymore, so we only clear the present bit in the PTE despite pmd_present still returns true (just like PROT_NONE, nothing new in this respect). pmd_present never meant the real present bit in the pte was set, it just means the pmd points to RAM. It means it doesn't point to swap or migration entry and you can do pmd_to_page and it works fine. We need to invalidate the TLB by clearing the present bit and by flushing the TLB before overwriting the transhuge pmd with the regular pte (i.e. to make it non huge). That is actually required by an errata (l1 cache aliasing of the same mapping through two different TLB of two different sizes broke some old CPU and triggered machine checks). It's not something fundamentally necessary from a common code point of view. It's more risky from an hardware (not software) standpoint and before you can get rid of the pmd you need to do a TLB flush anyway to be sure CPUs stops using it, so better clear the present bit before doing the real costly thing (the tlb flush with IPIs). Clearing the present bit during the TLB flush is a cost that gets lost in the noise. The clear of the real present bit during pmd (virtual) splitting is done with pmdp_invalidate, that is created specifically to keeps pmd_trans_huge=true, pmd_present=true despite the present bit is not set. So you could imagine _PAGE_PSE as the real present bit. Before the physical split was deferred and decoupled from the virtual memory pmd split, pmd_trans_splitting allowed to wait the split to finish and to keep all gup_fast at bay during it (while the page was still mapped readable and writable in userland by other CPUs). Now the physical split is deferred so you just split the pmd locally and only a physical split invoked on the page (not the virtual split invoked on the pmd with split_huge_pmd) has to keep gup at bay, and it does so by freezing the refcount so all gup_fast fail with the page_cache_get_speculative during the freeze. This removed the need of the pmd_splitting flag in gup_fast (when pmd_splitting was set gup fast had to go through the non-fast gup), but it means that now a hugepage cannot be physically splitted if it's gup pinned. The main difference is that freezing the refcount can fail, so the code must learn to cope with such failure and defer it. Decoupling the physical and virtual splits introduced the need of tracking the doublemap case with a new PG_double_map flag too. It makes the refcounting of hugepages trivial in comparison (identical to hugetlbfs in fact), but it requires total_mapcount to account for all those huge and non huge mappings. It primarily pays off to add THP to tmpfs where the physical split may have to be deferred for pagecache reasons anyway. Thanks, Andrea
Re: [RFC 00/60] Coscheduling for Linux
On Fri, Sep 07, 2018 at 11:39:47PM +0200, Jan H. Schönherr wrote: > C) How does it work? > > > This patch series introduces hierarchical runqueues, that represent larger > and larger fractions of the system. By default, there is one runqueue per > scheduling domain. These additional levels of runqueues are activated by > the "cosched_max_level=" kernel command line argument. The bottom level is > 0. > > One CPU per hierarchical runqueue is considered the leader, who is > primarily responsible for the scheduling decision at this level. Once the > leader has selected a task group to execute, it notifies all leaders of the > runqueues below it to select tasks/task groups within the selected task > group. > > For each task-group, the user can select at which level it should be > scheduled. If you set "cpu.scheduled" to "1", coscheduling will typically > happen at core-level on systems with SMT. That is, if one SMT sibling > executes a task from this task group, the other sibling will do so, too. If > no task is available, the SMT sibling will be idle. With "cpu.scheduled" > set to "2" this is extended to the next level, which is typically a whole > socket on many systems. And so on. If you feel, that this does not provide > enough flexibility, you can specify "cosched_split_domains" on the kernel > command line to create more fine-grained scheduling domains for your > system. Have you considered using cpuset to specify the set of CPUs inside which you want to coschedule task groups in? Perhaps that would be more flexible and intuitive to control than this cpu.scheduled value. Unless you require this feature to act always symmetrical through the branches of a given domain tree? Thanks.
Re: [RFC 00/60] Coscheduling for Linux
On Fri, Sep 07, 2018 at 11:39:47PM +0200, Jan H. Schönherr wrote: > C) How does it work? > > > This patch series introduces hierarchical runqueues, that represent larger > and larger fractions of the system. By default, there is one runqueue per > scheduling domain. These additional levels of runqueues are activated by > the "cosched_max_level=" kernel command line argument. The bottom level is > 0. > > One CPU per hierarchical runqueue is considered the leader, who is > primarily responsible for the scheduling decision at this level. Once the > leader has selected a task group to execute, it notifies all leaders of the > runqueues below it to select tasks/task groups within the selected task > group. > > For each task-group, the user can select at which level it should be > scheduled. If you set "cpu.scheduled" to "1", coscheduling will typically > happen at core-level on systems with SMT. That is, if one SMT sibling > executes a task from this task group, the other sibling will do so, too. If > no task is available, the SMT sibling will be idle. With "cpu.scheduled" > set to "2" this is extended to the next level, which is typically a whole > socket on many systems. And so on. If you feel, that this does not provide > enough flexibility, you can specify "cosched_split_domains" on the kernel > command line to create more fine-grained scheduling domains for your > system. Have you considered using cpuset to specify the set of CPUs inside which you want to coschedule task groups in? Perhaps that would be more flexible and intuitive to control than this cpu.scheduled value. Unless you require this feature to act always symmetrical through the branches of a given domain tree? Thanks.
[PATCH v6 0/2] spi: Add Macronix controller driver
From: Mason Yang Hi Mark & Boris, I patched v6 spi-mxic.c based on Boris's comments of v5. Thanks for your review. best regards, Mason Mason Yang (2): spi: Add MXIC controller driver dt-binding: spi: Document Macronix controller bindings Documentation/devicetree/bindings/spi/spi-mxic.txt | 34 ++ drivers/spi/Kconfig| 6 + drivers/spi/Makefile | 1 + drivers/spi/spi-mxic.c | 619 + 4 files changed, 660 insertions(+) create mode 100644 Documentation/devicetree/bindings/spi/spi-mxic.txt create mode 100644 drivers/spi/spi-mxic.c -- 1.9.1
[PATCH v6 1/2] spi: Add MXIC controller driver
From: Mason Yang Add a driver for Macronix SPI controller IP. Signed-off-by: Mason Yang --- drivers/spi/Kconfig| 6 + drivers/spi/Makefile | 1 + drivers/spi/spi-mxic.c | 619 + 3 files changed, 626 insertions(+) create mode 100644 drivers/spi/spi-mxic.c diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index ad5d68e..7e900b5 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -633,6 +633,12 @@ config SPI_SUN6I help This enables using the SPI controller on the Allwinner A31 SoCs. +config SPI_MXIC +tristate "Macronix MX25F0A SPI controller" +depends on SPI_MASTER +help + This selects the Macronix MX25F0A SPI controller driver. + config SPI_MXS tristate "Freescale MXS SPI controller" depends on ARCH_MXS diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index cb1f437..d7a1ceb 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -57,6 +57,7 @@ obj-$(CONFIG_SPI_MPC512x_PSC) += spi-mpc512x-psc.o obj-$(CONFIG_SPI_MPC52xx_PSC) += spi-mpc52xx-psc.o obj-$(CONFIG_SPI_MPC52xx) += spi-mpc52xx.o obj-$(CONFIG_SPI_MT65XX)+= spi-mt65xx.o +obj-$(CONFIG_SPI_MXIC) += spi-mxic.o obj-$(CONFIG_SPI_MXS) += spi-mxs.o obj-$(CONFIG_SPI_NUC900) += spi-nuc900.o obj-$(CONFIG_SPI_OC_TINY) += spi-oc-tiny.o diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c new file mode 100644 index 000..e41ae6e --- /dev/null +++ b/drivers/spi/spi-mxic.c @@ -0,0 +1,619 @@ +// SPDX-License-Identifier: GPL-2.0 +// +// Copyright (C) 2018 Macronix International Co., Ltd. +// +// Authors: +// Mason Yang +// zhengxunli +// Boris Brezillon +// + +#include +#include +#include +#include +#include +#include +#include +#include + +#define HC_CFG 0x0 +#define HC_CFG_IF_CFG(x) ((x) << 27) +#define HC_CFG_DUAL_SLAVE BIT(31) +#define HC_CFG_INDIVIDUAL BIT(30) +#define HC_CFG_NIO(x) (((x) / 4) << 27) +#define HC_CFG_TYPE(s, t) ((t) << (23 + ((s) * 2))) +#define HC_CFG_TYPE_SPI_NOR0 +#define HC_CFG_TYPE_SPI_NAND 1 +#define HC_CFG_TYPE_SPI_RAM2 +#define HC_CFG_TYPE_RAW_NAND 3 +#define HC_CFG_SLV_ACT(x) ((x) << 21) +#define HC_CFG_CLK_PH_EN BIT(20) +#define HC_CFG_CLK_POL_INV BIT(19) +#define HC_CFG_BIG_ENDIAN BIT(18) +#define HC_CFG_DATA_PASS BIT(17) +#define HC_CFG_IDLE_SIO_LVL(x) ((x) << 16) +#define HC_CFG_MAN_START_ENBIT(3) +#define HC_CFG_MAN_START BIT(2) +#define HC_CFG_MAN_CS_EN BIT(1) +#define HC_CFG_MAN_CS_ASSERT BIT(0) + +#define INT_STS0x4 +#define INT_STS_EN 0x8 +#define INT_SIG_EN 0xc +#define INT_STS_ALLGENMASK(31, 0) +#define INT_RDY_PINBIT(26) +#define INT_RDY_SR BIT(25) +#define INT_LNR_SUSP BIT(24) +#define INT_ECC_ERRBIT(17) +#define INT_CRC_ERRBIT(16) +#define INT_LWR_DISBIT(12) +#define INT_LRD_DISBIT(11) +#define INT_SDMA_INT BIT(10) +#define INT_DMA_FINISH BIT(9) +#define INT_RX_NOT_FULLBIT(3) +#define INT_RX_NOT_EMPTY BIT(2) +#define INT_TX_NOT_FULLBIT(1) +#define INT_TX_EMPTY BIT(0) + +#define HC_EN 0x10 +#define HC_EN_BIT BIT(0) + +#define TXD(x) (0x14 + ((x) * 4)) +#define RXD0x24 + +#define SS_CTRL(s) (0x30 + ((s) * 4)) +#define LRD_CFG0x44 +#define LWR_CFG0x80 +#define RWW_CFG0x70 +#define OP_READBIT(23) +#define OP_DUMMY_CYC(x)((x) << 17) +#define OP_ADDR_BYTES(x) ((x) << 14) +#define OP_CMD_BYTES(x)(((x) - 1) << 13) +#define OP_OCTA_CRC_EN BIT(12) +#define OP_DQS_EN BIT(11) +#define OP_ENHC_EN BIT(10) +#define OP_PREAMBLE_EN BIT(9) +#define OP_DATA_DDRBIT(8) +#define OP_DATA_BUSW(x)((x) << 6) +#define OP_ADDR_DDRBIT(5) +#define OP_ADDR_BUSW(x)((x) << 3) +#define OP_CMD_DDR BIT(2) +#define OP_CMD_BUSW(x) (x) +#define OP_BUSW_1 0 +#define OP_BUSW_2 1 +#define OP_BUSW_4 2 +#define OP_BUSW_8 3 + +#define OCTA_CRC 0x38 +#define OCTA_CRC_IN_EN(s) BIT(3 + ((s) * 16)) +#define OCTA_CRC_CHUNK(s, x) ((fls((x) / 32)) << (1 + ((s) * 16))) +#define OCTA_CRC_OUT_EN(s) BIT(0 + ((s) * 16)) + +#define ONFI_DIN_CNT(s)(0x3c + (s)) + +#define LRD_CTRL 0x48 +#define RWW_CTRL 0x74 +#define LWR_CTRL 0x84 +#define LMODE_EN BIT(31) +#define LMODE_SLV_ACT(x) ((x) << 21)
[PATCH v6 2/2] dt-binding: spi: Document Macronix controller bindings
From: Mason Yang
Document the bindings used by the Macronix controller.
Signed-off-by: Mason Yang
---
Documentation/devicetree/bindings/spi/spi-mxic.txt | 34 ++
1 file changed, 34 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-mxic.txt
diff --git a/Documentation/devicetree/bindings/spi/spi-mxic.txt
b/Documentation/devicetree/bindings/spi/spi-mxic.txt
new file mode 100644
index 000..529f2da
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-mxic.txt
@@ -0,0 +1,34 @@
+Macronix SPI controller Device Tree Bindings
+
+
+Required properties:
+- compatible: should be "mxicy,mx25f0a-spi"
+- #address-cells: should be 1
+- #size-cells: should be 0
+- reg: should contain 2 entries, one for the registers and one for the direct
+ mapping area
+- reg-names: should contain "regs" and "dirmap"
+- interrupts: interrupt line connected to the SPI controller
+- clock-names: should contain "ps_clk", "send_clk" and "send_dly_clk"
+- clocks: should contain 3 entries for the "ps_clk", "send_clk" and
+ "send_dly_clk" clocks
+
+Example:
+
+ spi@43c3 {
+ compatible = "mxicy,mx25f0a-spi";
+ reg = <0x43c3 0x1>, <0xa000 0x2000>;
+ reg-names = "regs", "dirmap";
+ clocks = < 0>, < 1>, < 18>;
+ clock-names = "send_clk", "send_dly_clk", "ps_clk";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ flash@0 {
+ compatible = "jedec,spi-nor";
+ reg = <0>;
+ spi-max-frequency = <2500>;
+ spi-tx-bus-width = <4>;
+ spi-rx-bus-width = <4>;
+ };
+ };
--
1.9.1
[PATCH v6 0/2] spi: Add Macronix controller driver
From: Mason Yang Hi Mark & Boris, I patched v6 spi-mxic.c based on Boris's comments of v5. Thanks for your review. best regards, Mason Mason Yang (2): spi: Add MXIC controller driver dt-binding: spi: Document Macronix controller bindings Documentation/devicetree/bindings/spi/spi-mxic.txt | 34 ++ drivers/spi/Kconfig| 6 + drivers/spi/Makefile | 1 + drivers/spi/spi-mxic.c | 619 + 4 files changed, 660 insertions(+) create mode 100644 Documentation/devicetree/bindings/spi/spi-mxic.txt create mode 100644 drivers/spi/spi-mxic.c -- 1.9.1
