Re: [PATCH v4 2/2] tpm: add support for partial reads

[Jarkko Sakkinen]

On Fri, Nov 16, 2018 at 09:51:51AM -0800, Tadeusz Struk wrote:
> Currently to read a response from the TPM device an application needs
> provide big enough buffer for the whole response and read it in one go.
> The application doesn't know how big the response it beforehand so it
> always needs to maintain a 4K buffer and read the max (4K).
> In case if the user of the TSS library doesn't provide big enough
> buffer the TCTI spec says that the library should set the required
> size and return TSS2_TCTI_RC_INSUFFICIENT_BUFFER error code so that the
> application could allocate a bigger buffer and call receive again.
> To make it possible in the TSS library, this requires being able to do
> partial reads from the driver.
> The library would read the 10 bytes header first to get the actual size
> of the response from the header, and then read the rest of the response.
> 
> This patch adds support for partial reads, i.e. the user can read the
> response in one or multiple reads, until the whole response is consumed.
> The user can also read only part of the response and ignore
> the rest by issuing a new write to send a new command.
> 
> Signed-off-by: Tadeusz Struk 
> ---
> The usecase is implemented in this TSS commit:
> https://github.com/tpm2-software/tpm2-tss/commit/ce982f67a67dc08e24683d30b05800648d8a264c
> 
> Changes in v4:
>  - Use unsigned type for response_pending as it will never be negative.
>  - Rebased on top of name change data_pending to transmit_result patch.
> 
> Changes in v3:
>  - Remove link to usecase implemented in TSS out of the commit message.
>  - Update the conddition in tpm_common_poll() to take into account
>the partial_data also.
> 
> Changes in v2:
>  - Allow writes after only partial response is consumed to maintain
>backwords compatibility.
> ---
>  drivers/char/tpm/tpm-dev-common.c |   41 
> -
>  drivers/char/tpm/tpm-dev.h|2 ++
>  2 files changed, 33 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-dev-common.c 
> b/drivers/char/tpm/tpm-dev-common.c
> index 67a70e2fde7f..0544733e1b6d 100644
> --- a/drivers/char/tpm/tpm-dev-common.c
> +++ b/drivers/char/tpm/tpm-dev-common.c
> @@ -64,6 +64,7 @@ static void tpm_timeout_work(struct work_struct *work)
>  
>   mutex_lock(>buffer_mutex);
>   priv->transmit_result = 0;
> + priv->partial_data = 0;
>   memset(priv->data_buffer, 0, sizeof(priv->data_buffer));
>   mutex_unlock(>buffer_mutex);
>   wake_up_interruptible(>async_wait);
> @@ -90,22 +91,39 @@ ssize_t tpm_common_read(struct file *file, char __user 
> *buf,
>   ssize_t ret_size = 0;
>   int rc;
>  
> - del_singleshot_timer_sync(>user_read_timer);
> - flush_work(>timeout_work);
>   mutex_lock(>buffer_mutex);
> + if (priv->transmit_result || priv->partial_data) {
> + if (*off == 0)
> + priv->partial_data = priv->transmit_result;
> +
> + ret_size = min_t(ssize_t, size, priv->partial_data);
> + if (ret_size <= 0) {

When ret_size < 0? Shouldn't this be just "if (!ret_size)"?

> + ret_size = 0;
> + priv->transmit_result = 0;
> + priv->partial_data = 0;
> + goto out;
> + }
>  
> - if (priv->transmit_result) {
> - ret_size = min_t(ssize_t, size, priv->transmit_result);
> - if (ret_size > 0) {
> - rc = copy_to_user(buf, priv->data_buffer, ret_size);
> - memset(priv->data_buffer, 0, priv->transmit_result);
> - if (rc)
> - ret_size = -EFAULT;
> + rc = copy_to_user(buf, priv->data_buffer + *off, ret_size);
> + if (rc) {
> + memset(priv->data_buffer, 0, TPM_BUFSIZE);
> + priv->partial_data = 0;
> + ret_size = -EFAULT;
> + } else {
> + memset(priv->data_buffer + *off, 0, ret_size);
> + priv->partial_data -= ret_size;
> + *off += ret_size;
>   }
>  
>   priv->transmit_result = 0;
>   }
>  
> +out:
> + if (!priv->partial_data) {
> + *off = 0;
> + del_singleshot_timer_sync(>user_read_timer);
> + flush_work(>timeout_work);
> + }
>   mutex_unlock(>buffer_mutex);
>   return ret_size;
>  }
> @@ -150,6 +168,9 @@ ssize_t tpm_common_write(struct file *file, const char 
> __user *buf,
>   goto out;
>   }
>  
> + priv->partial_data = 0;
> + *off = 0;
> +
>   /*
>* If in nonblocking mode schedule an async job to send
>* the command return the size.
> @@ -184,7 +205,7 @@ __poll_t tpm_common_poll(struct file *file, poll_table 
> *wait)
>  
>   poll_wait(file, >async_wait, wait);
>  
> - if (priv->transmit_result)
> + if 

Re: [PATCH v4 2/2] tpm: add support for partial reads

[Jarkko Sakkinen]

On Fri, Nov 16, 2018 at 09:51:51AM -0800, Tadeusz Struk wrote:
> Currently to read a response from the TPM device an application needs
> provide big enough buffer for the whole response and read it in one go.
> The application doesn't know how big the response it beforehand so it
> always needs to maintain a 4K buffer and read the max (4K).
> In case if the user of the TSS library doesn't provide big enough
> buffer the TCTI spec says that the library should set the required
> size and return TSS2_TCTI_RC_INSUFFICIENT_BUFFER error code so that the
> application could allocate a bigger buffer and call receive again.
> To make it possible in the TSS library, this requires being able to do
> partial reads from the driver.
> The library would read the 10 bytes header first to get the actual size
> of the response from the header, and then read the rest of the response.
> 
> This patch adds support for partial reads, i.e. the user can read the
> response in one or multiple reads, until the whole response is consumed.
> The user can also read only part of the response and ignore
> the rest by issuing a new write to send a new command.
> 
> Signed-off-by: Tadeusz Struk 
> ---
> The usecase is implemented in this TSS commit:
> https://github.com/tpm2-software/tpm2-tss/commit/ce982f67a67dc08e24683d30b05800648d8a264c
> 
> Changes in v4:
>  - Use unsigned type for response_pending as it will never be negative.
>  - Rebased on top of name change data_pending to transmit_result patch.
> 
> Changes in v3:
>  - Remove link to usecase implemented in TSS out of the commit message.
>  - Update the conddition in tpm_common_poll() to take into account
>the partial_data also.
> 
> Changes in v2:
>  - Allow writes after only partial response is consumed to maintain
>backwords compatibility.
> ---
>  drivers/char/tpm/tpm-dev-common.c |   41 
> -
>  drivers/char/tpm/tpm-dev.h|2 ++
>  2 files changed, 33 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-dev-common.c 
> b/drivers/char/tpm/tpm-dev-common.c
> index 67a70e2fde7f..0544733e1b6d 100644
> --- a/drivers/char/tpm/tpm-dev-common.c
> +++ b/drivers/char/tpm/tpm-dev-common.c
> @@ -64,6 +64,7 @@ static void tpm_timeout_work(struct work_struct *work)
>  
>   mutex_lock(>buffer_mutex);
>   priv->transmit_result = 0;
> + priv->partial_data = 0;
>   memset(priv->data_buffer, 0, sizeof(priv->data_buffer));
>   mutex_unlock(>buffer_mutex);
>   wake_up_interruptible(>async_wait);
> @@ -90,22 +91,39 @@ ssize_t tpm_common_read(struct file *file, char __user 
> *buf,
>   ssize_t ret_size = 0;
>   int rc;
>  
> - del_singleshot_timer_sync(>user_read_timer);
> - flush_work(>timeout_work);
>   mutex_lock(>buffer_mutex);
> + if (priv->transmit_result || priv->partial_data) {
> + if (*off == 0)
> + priv->partial_data = priv->transmit_result;
> +
> + ret_size = min_t(ssize_t, size, priv->partial_data);
> + if (ret_size <= 0) {

When ret_size < 0? Shouldn't this be just "if (!ret_size)"?

> + ret_size = 0;
> + priv->transmit_result = 0;
> + priv->partial_data = 0;
> + goto out;
> + }
>  
> - if (priv->transmit_result) {
> - ret_size = min_t(ssize_t, size, priv->transmit_result);
> - if (ret_size > 0) {
> - rc = copy_to_user(buf, priv->data_buffer, ret_size);
> - memset(priv->data_buffer, 0, priv->transmit_result);
> - if (rc)
> - ret_size = -EFAULT;
> + rc = copy_to_user(buf, priv->data_buffer + *off, ret_size);
> + if (rc) {
> + memset(priv->data_buffer, 0, TPM_BUFSIZE);
> + priv->partial_data = 0;
> + ret_size = -EFAULT;
> + } else {
> + memset(priv->data_buffer + *off, 0, ret_size);
> + priv->partial_data -= ret_size;
> + *off += ret_size;
>   }
>  
>   priv->transmit_result = 0;
>   }
>  
> +out:
> + if (!priv->partial_data) {
> + *off = 0;
> + del_singleshot_timer_sync(>user_read_timer);
> + flush_work(>timeout_work);
> + }
>   mutex_unlock(>buffer_mutex);
>   return ret_size;
>  }
> @@ -150,6 +168,9 @@ ssize_t tpm_common_write(struct file *file, const char 
> __user *buf,
>   goto out;
>   }
>  
> + priv->partial_data = 0;
> + *off = 0;
> +
>   /*
>* If in nonblocking mode schedule an async job to send
>* the command return the size.
> @@ -184,7 +205,7 @@ __poll_t tpm_common_poll(struct file *file, poll_table 
> *wait)
>  
>   poll_wait(file, >async_wait, wait);
>  
> - if (priv->transmit_result)
> + if 

Re: [PATCH 1/2] tpm: rename data_pending to transmit_result

[Jarkko Sakkinen]

On Fri, Nov 16, 2018 at 09:51:13AM -0800, Tadeusz Struk wrote:
> The file_priv->data_pending name is not adequate as it
> doesn't contain any data, but the result from the last
> successful call to tpm_transmit() function, so rename it
> to transmit_result. Also its type should be size_t instead
> of ssize_t as it will never be negative. Change it as well.
> 
> Signed-off-by: Tadeusz Struk 
> ---
>  drivers/char/tpm/tpm-dev-common.c |   20 ++--
>  drivers/char/tpm/tpm-dev.h|4 ++--
>  2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-dev-common.c 
> b/drivers/char/tpm/tpm-dev-common.c
> index 99b5133a9d05..67a70e2fde7f 100644
> --- a/drivers/char/tpm/tpm-dev-common.c
> +++ b/drivers/char/tpm/tpm-dev-common.c
> @@ -40,7 +40,7 @@ static void tpm_async_work(struct work_struct *work)
>  
>   tpm_put_ops(priv->chip);
>   if (ret > 0) {
> - priv->data_pending = ret;
> + priv->transmit_result = ret;

I don't know if you saw my later response before sending this but
since you made the remark that it does not carry the error code
more proper name is response_length because that is exactly the
data it contains.

/Jarkko

Re: [PATCH 1/2] tpm: rename data_pending to transmit_result

[Jarkko Sakkinen]

On Fri, Nov 16, 2018 at 09:51:13AM -0800, Tadeusz Struk wrote:
> The file_priv->data_pending name is not adequate as it
> doesn't contain any data, but the result from the last
> successful call to tpm_transmit() function, so rename it
> to transmit_result. Also its type should be size_t instead
> of ssize_t as it will never be negative. Change it as well.
> 
> Signed-off-by: Tadeusz Struk 
> ---
>  drivers/char/tpm/tpm-dev-common.c |   20 ++--
>  drivers/char/tpm/tpm-dev.h|4 ++--
>  2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-dev-common.c 
> b/drivers/char/tpm/tpm-dev-common.c
> index 99b5133a9d05..67a70e2fde7f 100644
> --- a/drivers/char/tpm/tpm-dev-common.c
> +++ b/drivers/char/tpm/tpm-dev-common.c
> @@ -40,7 +40,7 @@ static void tpm_async_work(struct work_struct *work)
>  
>   tpm_put_ops(priv->chip);
>   if (ret > 0) {
> - priv->data_pending = ret;
> + priv->transmit_result = ret;

I don't know if you saw my later response before sending this but
since you made the remark that it does not carry the error code
more proper name is response_length because that is exactly the
data it contains.

/Jarkko

Re: [PATCH v8 16/17] tpm: take TPM chip power gating out of tpm_transmit()

[Jarkko Sakkinen]

On Fri, Nov 16, 2018 at 12:02:17PM -0500, Stefan Berger wrote:
> On 11/16/18 7:38 AM, Jarkko Sakkinen wrote:
> > Call tpm_chip_start() and tpm_chip_stop() in
> > 
> > * tpm_try_get_ops() and tpm_put_ops()
> > * tpm_chip_register()
> > * tpm2_del_space()
> > 
> > And remove these calls from tpm_transmit(). The core reason for this
> > change is that in tpm_vtpm_proxy a locality change requires a virtual
> > TPM command (a command made up just for that driver).
> > 
> > The consequence of this is that this commit removes the remaining nested
> > calls.
> > 
> > Signed-off-by: Jarkko Sakkinen 
> > ---
> >   drivers/char/tpm/tpm-chip.c   | 21 -
> >   drivers/char/tpm/tpm-interface.c  |  4 
> >   drivers/char/tpm/tpm.h|  9 -
> >   drivers/char/tpm/tpm2-space.c |  5 -
> >   drivers/char/tpm/tpm_vtpm_proxy.c |  3 +--
> >   5 files changed, 13 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > index 65f1561eba81..837d44fa0797 100644
> > --- a/drivers/char/tpm/tpm-chip.c
> > +++ b/drivers/char/tpm/tpm-chip.c
> > @@ -41,9 +41,6 @@ static int tpm_request_locality(struct tpm_chip *chip, 
> > unsigned int flags)
> >   {
> > int rc;
> > 
> > -   if (flags & TPM_TRANSMIT_NESTED)
> > -   return 0;
> > -
> > if (!chip->ops->request_locality)
> > return 0;
> > 
> > @@ -59,9 +56,6 @@ static void tpm_relinquish_locality(struct tpm_chip 
> > *chip, unsigned int flags)
> >   {
> > int rc;
> > 
> > -   if (flags & TPM_TRANSMIT_NESTED)
> > -   return;
> > -
> > if (!chip->ops->relinquish_locality)
> > return;
> > 
> > @@ -74,9 +68,6 @@ static void tpm_relinquish_locality(struct tpm_chip 
> > *chip, unsigned int flags)
> > 
> >   static int tpm_cmd_ready(struct tpm_chip *chip, unsigned int flags)
> >   {
> > -   if (flags & TPM_TRANSMIT_NESTED)
> > -   return 0;
> > -
> > if (!chip->ops->cmd_ready)
> > return 0;
> > 
> > @@ -85,9 +76,6 @@ static int tpm_cmd_ready(struct tpm_chip *chip, unsigned 
> > int flags)
> > 
> >   static int tpm_go_idle(struct tpm_chip *chip, unsigned int flags)
> >   {
> > -   if (flags & TPM_TRANSMIT_NESTED)
> > -   return 0;
> > -
> > if (!chip->ops->go_idle)
> > return 0;
> > 
> > @@ -169,7 +157,9 @@ int tpm_try_get_ops(struct tpm_chip *chip)
> > goto out_lock;
> > 
> > mutex_lock(>tpm_mutex);
> > -   return 0;
> > +   rc = tpm_chip_start(chip, 0);
> > +   if (rc)
> > +   mutex_unlock(>tpm_mutex);
> 
> This cannot be right to fall through to up_read() etc.

Ouch! It is not and I had a fixup for it that was not applied to the
resulting patch set :-/

/Jarkko

Re: [PATCH v8 16/17] tpm: take TPM chip power gating out of tpm_transmit()

[Jarkko Sakkinen]

On Fri, Nov 16, 2018 at 12:02:17PM -0500, Stefan Berger wrote:
> On 11/16/18 7:38 AM, Jarkko Sakkinen wrote:
> > Call tpm_chip_start() and tpm_chip_stop() in
> > 
> > * tpm_try_get_ops() and tpm_put_ops()
> > * tpm_chip_register()
> > * tpm2_del_space()
> > 
> > And remove these calls from tpm_transmit(). The core reason for this
> > change is that in tpm_vtpm_proxy a locality change requires a virtual
> > TPM command (a command made up just for that driver).
> > 
> > The consequence of this is that this commit removes the remaining nested
> > calls.
> > 
> > Signed-off-by: Jarkko Sakkinen 
> > ---
> >   drivers/char/tpm/tpm-chip.c   | 21 -
> >   drivers/char/tpm/tpm-interface.c  |  4 
> >   drivers/char/tpm/tpm.h|  9 -
> >   drivers/char/tpm/tpm2-space.c |  5 -
> >   drivers/char/tpm/tpm_vtpm_proxy.c |  3 +--
> >   5 files changed, 13 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > index 65f1561eba81..837d44fa0797 100644
> > --- a/drivers/char/tpm/tpm-chip.c
> > +++ b/drivers/char/tpm/tpm-chip.c
> > @@ -41,9 +41,6 @@ static int tpm_request_locality(struct tpm_chip *chip, 
> > unsigned int flags)
> >   {
> > int rc;
> > 
> > -   if (flags & TPM_TRANSMIT_NESTED)
> > -   return 0;
> > -
> > if (!chip->ops->request_locality)
> > return 0;
> > 
> > @@ -59,9 +56,6 @@ static void tpm_relinquish_locality(struct tpm_chip 
> > *chip, unsigned int flags)
> >   {
> > int rc;
> > 
> > -   if (flags & TPM_TRANSMIT_NESTED)
> > -   return;
> > -
> > if (!chip->ops->relinquish_locality)
> > return;
> > 
> > @@ -74,9 +68,6 @@ static void tpm_relinquish_locality(struct tpm_chip 
> > *chip, unsigned int flags)
> > 
> >   static int tpm_cmd_ready(struct tpm_chip *chip, unsigned int flags)
> >   {
> > -   if (flags & TPM_TRANSMIT_NESTED)
> > -   return 0;
> > -
> > if (!chip->ops->cmd_ready)
> > return 0;
> > 
> > @@ -85,9 +76,6 @@ static int tpm_cmd_ready(struct tpm_chip *chip, unsigned 
> > int flags)
> > 
> >   static int tpm_go_idle(struct tpm_chip *chip, unsigned int flags)
> >   {
> > -   if (flags & TPM_TRANSMIT_NESTED)
> > -   return 0;
> > -
> > if (!chip->ops->go_idle)
> > return 0;
> > 
> > @@ -169,7 +157,9 @@ int tpm_try_get_ops(struct tpm_chip *chip)
> > goto out_lock;
> > 
> > mutex_lock(>tpm_mutex);
> > -   return 0;
> > +   rc = tpm_chip_start(chip, 0);
> > +   if (rc)
> > +   mutex_unlock(>tpm_mutex);
> 
> This cannot be right to fall through to up_read() etc.

Ouch! It is not and I had a fixup for it that was not applied to the
resulting patch set :-/

/Jarkko

Re: [PATCH v8 08/17] tpm: call tpm2_flush_space() on error in tpm_try_transmit()

[Jarkko Sakkinen]

On Fri, Nov 16, 2018 at 11:19:57AM -0500, Sasha Levin wrote:
> On Fri, Nov 16, 2018 at 02:38:32PM +0200, Jarkko Sakkinen wrote:
> > Always call tpm2_flush_space() on failure in tpm_try_transmit() so that
> > the volatile memory of the TPM gets cleared. If /dev/tpm0 does not have
> > sufficient permissions (usually it has), this could lead to the leakage
> > of TPM objects. Through /dev/tpmrm0 this issue does not raise any new
> > security concerns.
> > 
> > Cc: James Bottomley 
> > Cc: sta...@vger.kernel.org
> > Fixes: 745b361e989a ("tpm:tpm: infrastructure for TPM spaces")
> > Signed-off-by: Jarkko Sakkinen 
> > Reviewed-by: Stefan Berger 
> 
> Hi Jarkko,
> 
> This patch seems to depend on previous patches in this series, but those
> were not tagged for stable. Do they also need to be backported? If so,
> can you tag them as such?

Hi

Is that the preferred approach?

I've usually followed this workflow:

1. Mark patches with a fix to a regression with the fixes tag.
2. If a merge conflict raises, I'll locate the deps.

I've done it this way because often patches can depend on patches
outside the patch set. Anyway, I'm open to change my workflow if
that is required.

/Jarkko

Re: [PATCH v8 08/17] tpm: call tpm2_flush_space() on error in tpm_try_transmit()

[Jarkko Sakkinen]

On Fri, Nov 16, 2018 at 11:19:57AM -0500, Sasha Levin wrote:
> On Fri, Nov 16, 2018 at 02:38:32PM +0200, Jarkko Sakkinen wrote:
> > Always call tpm2_flush_space() on failure in tpm_try_transmit() so that
> > the volatile memory of the TPM gets cleared. If /dev/tpm0 does not have
> > sufficient permissions (usually it has), this could lead to the leakage
> > of TPM objects. Through /dev/tpmrm0 this issue does not raise any new
> > security concerns.
> > 
> > Cc: James Bottomley 
> > Cc: sta...@vger.kernel.org
> > Fixes: 745b361e989a ("tpm:tpm: infrastructure for TPM spaces")
> > Signed-off-by: Jarkko Sakkinen 
> > Reviewed-by: Stefan Berger 
> 
> Hi Jarkko,
> 
> This patch seems to depend on previous patches in this series, but those
> were not tagged for stable. Do they also need to be backported? If so,
> can you tag them as such?

Hi

Is that the preferred approach?

I've usually followed this workflow:

1. Mark patches with a fix to a regression with the fixes tag.
2. If a merge conflict raises, I'll locate the deps.

I've done it this way because often patches can depend on patches
outside the patch set. Anyway, I'm open to change my workflow if
that is required.

/Jarkko

Re: [PATCH v5 6/7] tpm: ensure that the output of PCR read contains the correct digest size

[Jarkko Sakkinen]

On Fri, Nov 16, 2018 at 05:06:48PM +0100, Roberto Sassu wrote:
> On 11/16/2018 2:41 PM, Jarkko Sakkinen wrote:
> > On Wed, Nov 14, 2018 at 04:31:07PM +0100, Roberto Sassu wrote:
> > > This patch protects against data corruption that could happen in the bus,
> > > by checking that that the digest size returned by the TPM during a PCR 
> > > read
> > > matches the size of the algorithm passed to tpm2_pcr_read().
> > > 
> > > This check is performed after information about the PCR banks has been
> > > retrieved.
> > > 
> > > Signed-off-by: Roberto Sassu 
> > > Reviewed-by: Jarkko Sakkinen 
> > > Cc: sta...@vger.kernel.org
> > 
> > Missing fixes tag.
> 
> Before this patch set, tpm2_pcr_extend() always copied 20 bytes from the
> output sent by the TPM.
> 
> Roberto

Aah, right, of course. Well the patch set is ATM somewhat broken because
this would require a fixes tag that points to a patch insdie the patch
set.

Probably good way to fix the issue is to just merge this with the
earlier commit.

/Jarkko

Re: [PATCH v5 6/7] tpm: ensure that the output of PCR read contains the correct digest size

[Jarkko Sakkinen]

On Fri, Nov 16, 2018 at 05:06:48PM +0100, Roberto Sassu wrote:
> On 11/16/2018 2:41 PM, Jarkko Sakkinen wrote:
> > On Wed, Nov 14, 2018 at 04:31:07PM +0100, Roberto Sassu wrote:
> > > This patch protects against data corruption that could happen in the bus,
> > > by checking that that the digest size returned by the TPM during a PCR 
> > > read
> > > matches the size of the algorithm passed to tpm2_pcr_read().
> > > 
> > > This check is performed after information about the PCR banks has been
> > > retrieved.
> > > 
> > > Signed-off-by: Roberto Sassu 
> > > Reviewed-by: Jarkko Sakkinen 
> > > Cc: sta...@vger.kernel.org
> > 
> > Missing fixes tag.
> 
> Before this patch set, tpm2_pcr_extend() always copied 20 bytes from the
> output sent by the TPM.
> 
> Roberto

Aah, right, of course. Well the patch set is ATM somewhat broken because
this would require a fixes tag that points to a patch insdie the patch
set.

Probably good way to fix the issue is to just merge this with the
earlier commit.

/Jarkko

Re: [PATCH v5 7/7] tpm: pass an array of tpm_bank_list structures to tpm_pcr_extend()

[Jarkko Sakkinen]

On Fri, Nov 16, 2018 at 04:55:36PM +0100, Roberto Sassu wrote:
> I understood from a previous email that you want to include all API
> changes for crypto agility in the same patch set.

Hmm.. maybe there is some misunderstading. Can you point me to the
comment? Thanks.

/Jarkko

Re: [PATCH v5 7/7] tpm: pass an array of tpm_bank_list structures to tpm_pcr_extend()

[Jarkko Sakkinen]

On Fri, Nov 16, 2018 at 04:55:36PM +0100, Roberto Sassu wrote:
> I understood from a previous email that you want to include all API
> changes for crypto agility in the same patch set.

Hmm.. maybe there is some misunderstading. Can you point me to the
comment? Thanks.

/Jarkko

Re: [PATCH v5 7/7] tpm: pass an array of tpm_bank_list structures to tpm_pcr_extend()

[Jarkko Sakkinen]

On Fri, Nov 16, 2018 at 04:55:36PM +0100, Roberto Sassu wrote:
> On 11/16/2018 4:03 PM, Jarkko Sakkinen wrote:
> > On Wed, Nov 14, 2018 at 04:31:08PM +0100, Roberto Sassu wrote:
> > > Currently, tpm_pcr_extend() accepts as an input only a SHA1 digest.
> > > 
> > > This patch modifies the definition of tpm_pcr_extend() to allow other
> > > kernel subsystems to pass a digest for each algorithm supported by the 
> > > TPM.
> > > All digests are processed by the TPM in one operation.
> > > 
> > > If a tpm_pcr_extend() caller provides a subset of the supported 
> > > algorithms,
> > > the TPM driver extends the remaining PCR banks with the first digest
> > > passed as an argument to the function.
> > 
> > What is the legit use case for this?
> 
> A subset could be chosen for better performance, or when a TPM algorithm
> is not supported by the crypto subsystem.

Doesn't extending a subset a security concern?

/Jarkko

Re: [PATCH v5 7/7] tpm: pass an array of tpm_bank_list structures to tpm_pcr_extend()

[Jarkko Sakkinen]

On Fri, Nov 16, 2018 at 04:55:36PM +0100, Roberto Sassu wrote:
> On 11/16/2018 4:03 PM, Jarkko Sakkinen wrote:
> > On Wed, Nov 14, 2018 at 04:31:08PM +0100, Roberto Sassu wrote:
> > > Currently, tpm_pcr_extend() accepts as an input only a SHA1 digest.
> > > 
> > > This patch modifies the definition of tpm_pcr_extend() to allow other
> > > kernel subsystems to pass a digest for each algorithm supported by the 
> > > TPM.
> > > All digests are processed by the TPM in one operation.
> > > 
> > > If a tpm_pcr_extend() caller provides a subset of the supported 
> > > algorithms,
> > > the TPM driver extends the remaining PCR banks with the first digest
> > > passed as an argument to the function.
> > 
> > What is the legit use case for this?
> 
> A subset could be chosen for better performance, or when a TPM algorithm
> is not supported by the crypto subsystem.

Doesn't extending a subset a security concern?

/Jarkko

Re: RFC: userspace exception fixups

[Jarkko Sakkinen]

On Sun, Nov 18, 2018 at 09:15:48AM +0200, Jarkko Sakkinen wrote:
> On Thu, Nov 01, 2018 at 10:53:40AM -0700, Andy Lutomirski wrote:
> > Hi all-
> > 
> > The people working on SGX enablement are grappling with a somewhat
> > annoying issue: the x86 EENTER instruction is used from user code and
> > can, as part of its normal-ish operation, raise an exception.  It is
> > also highly likely to be used from a library, and signal handling in
> > libraries is unpleasant at best.
> > 
> > There's been some discussion of adding a vDSO entry point to wrap
> > EENTER and do something sensible with the exceptions, but I'm
> > wondering if a more general mechanism would be helpful.
> 
> I haven't really followed all of this discussion because I've been busy
> working on the patch set but for me all of these approaches look awfully
> complicated.
> 
> I'll throw my own suggestion and apologize if this has been already
> suggested and discarded: return-to-AEP.
> 
> My idea is to do just a small extension to SGX AEX handling. At the
> moment hardware will RAX, RBX and RCX with ERESUME parameters. We can
> fill extend this by filling other three spare registers with exception

s/fill extend/extend/

/Jarkko

Re: RFC: userspace exception fixups

[Jarkko Sakkinen]

On Sun, Nov 18, 2018 at 09:15:48AM +0200, Jarkko Sakkinen wrote:
> On Thu, Nov 01, 2018 at 10:53:40AM -0700, Andy Lutomirski wrote:
> > Hi all-
> > 
> > The people working on SGX enablement are grappling with a somewhat
> > annoying issue: the x86 EENTER instruction is used from user code and
> > can, as part of its normal-ish operation, raise an exception.  It is
> > also highly likely to be used from a library, and signal handling in
> > libraries is unpleasant at best.
> > 
> > There's been some discussion of adding a vDSO entry point to wrap
> > EENTER and do something sensible with the exceptions, but I'm
> > wondering if a more general mechanism would be helpful.
> 
> I haven't really followed all of this discussion because I've been busy
> working on the patch set but for me all of these approaches look awfully
> complicated.
> 
> I'll throw my own suggestion and apologize if this has been already
> suggested and discarded: return-to-AEP.
> 
> My idea is to do just a small extension to SGX AEX handling. At the
> moment hardware will RAX, RBX and RCX with ERESUME parameters. We can
> fill extend this by filling other three spare registers with exception

s/fill extend/extend/

/Jarkko

Re: RFC: userspace exception fixups

[Jarkko Sakkinen]

On Thu, Nov 01, 2018 at 10:53:40AM -0700, Andy Lutomirski wrote:
> Hi all-
> 
> The people working on SGX enablement are grappling with a somewhat
> annoying issue: the x86 EENTER instruction is used from user code and
> can, as part of its normal-ish operation, raise an exception.  It is
> also highly likely to be used from a library, and signal handling in
> libraries is unpleasant at best.
> 
> There's been some discussion of adding a vDSO entry point to wrap
> EENTER and do something sensible with the exceptions, but I'm
> wondering if a more general mechanism would be helpful.

I haven't really followed all of this discussion because I've been busy
working on the patch set but for me all of these approaches look awfully
complicated.

I'll throw my own suggestion and apologize if this has been already
suggested and discarded: return-to-AEP.

My idea is to do just a small extension to SGX AEX handling. At the
moment hardware will RAX, RBX and RCX with ERESUME parameters. We can
fill extend this by filling other three spare registers with exception
information.

AEP handler can then do whatever it wants to do with this information
or just do ERESUME.

In some ways this dummied version of Sean's suggestion.

I think whatever the solution is it should be lightweight and this is
such solution. Why? Because exception handling could be then used to
implement other stuff than just error hadling like syscall wrapper
for the enclaves in nice and lean way.

/Jarkko

Re: RFC: userspace exception fixups

[Jarkko Sakkinen]

On Thu, Nov 01, 2018 at 10:53:40AM -0700, Andy Lutomirski wrote:
> Hi all-
> 
> The people working on SGX enablement are grappling with a somewhat
> annoying issue: the x86 EENTER instruction is used from user code and
> can, as part of its normal-ish operation, raise an exception.  It is
> also highly likely to be used from a library, and signal handling in
> libraries is unpleasant at best.
> 
> There's been some discussion of adding a vDSO entry point to wrap
> EENTER and do something sensible with the exceptions, but I'm
> wondering if a more general mechanism would be helpful.

I haven't really followed all of this discussion because I've been busy
working on the patch set but for me all of these approaches look awfully
complicated.

I'll throw my own suggestion and apologize if this has been already
suggested and discarded: return-to-AEP.

My idea is to do just a small extension to SGX AEX handling. At the
moment hardware will RAX, RBX and RCX with ERESUME parameters. We can
fill extend this by filling other three spare registers with exception
information.

AEP handler can then do whatever it wants to do with this information
or just do ERESUME.

In some ways this dummied version of Sean's suggestion.

I think whatever the solution is it should be lightweight and this is
such solution. Why? Because exception handling could be then used to
implement other stuff than just error hadling like syscall wrapper
for the enclaves in nice and lean way.

/Jarkko

Re: [PATCH v1 2/2] i2c: i2c-stm32f7: SYSCFG Fast Mode Plus support for I2C STM32F7

[kbuild test robot]

Hi Pierre-Yves,

I love your patch! Yet something to improve:

[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on v4.20-rc2 next-20181116]
[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/Pierre-Yves-MORDRET/SYSCFG-Fast-Mode-Plus-support-for-I2C-STM32F7/20181118-135056
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
i2c/for-next
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.1.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=8.1.0 make.cross ARCH=xtensa 

All errors (new ones prefixed by >>):

   drivers/i2c//busses/i2c-stm32f7.c: In function 
'stm32f7_i2c_setup_fm_plus_bits':
>> drivers/i2c//busses/i2c-stm32f7.c:1790:9: error: implicit declaration of 
>> function 'regmap_update_bits'; did you mean 'file_update_time'? 
>> [-Werror=implicit-function-declaration]
 return regmap_update_bits(i2c_dev->regmap, reg, mask, mask);
^~
file_update_time
   cc1: some warnings being treated as errors

vim +1790 drivers/i2c//busses/i2c-stm32f7.c

  1768  
  1769  static int stm32f7_i2c_setup_fm_plus_bits(struct platform_device *pdev,
  1770struct stm32f7_i2c_dev 
*i2c_dev)
  1771  {
  1772  struct device_node *np = pdev->dev.of_node;
  1773  int ret;
  1774  u32 reg, mask;
  1775  
  1776  i2c_dev->regmap = syscon_regmap_lookup_by_phandle(np, 
"st,syscfg-fmp");
  1777  if (IS_ERR(i2c_dev->regmap)) {
  1778  /* Optional */
  1779  return 0;
  1780  }
  1781  
  1782  ret = of_property_read_u32_index(np, "st,syscfg-fmp", 1, );
  1783  if (ret)
  1784  return ret;
  1785  
  1786  ret = of_property_read_u32_index(np, "st,syscfg-fmp", 2, );
  1787  if (ret)
  1788  return ret;
  1789  
> 1790  return regmap_update_bits(i2c_dev->regmap, reg, mask, mask);
  1791  }
  1792  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v1 2/2] i2c: i2c-stm32f7: SYSCFG Fast Mode Plus support for I2C STM32F7

[kbuild test robot]

Hi Pierre-Yves,

I love your patch! Yet something to improve:

[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on v4.20-rc2 next-20181116]
[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/Pierre-Yves-MORDRET/SYSCFG-Fast-Mode-Plus-support-for-I2C-STM32F7/20181118-135056
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
i2c/for-next
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.1.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=8.1.0 make.cross ARCH=xtensa 

All errors (new ones prefixed by >>):

   drivers/i2c//busses/i2c-stm32f7.c: In function 
'stm32f7_i2c_setup_fm_plus_bits':
>> drivers/i2c//busses/i2c-stm32f7.c:1790:9: error: implicit declaration of 
>> function 'regmap_update_bits'; did you mean 'file_update_time'? 
>> [-Werror=implicit-function-declaration]
 return regmap_update_bits(i2c_dev->regmap, reg, mask, mask);
^~
file_update_time
   cc1: some warnings being treated as errors

vim +1790 drivers/i2c//busses/i2c-stm32f7.c

  1768  
  1769  static int stm32f7_i2c_setup_fm_plus_bits(struct platform_device *pdev,
  1770struct stm32f7_i2c_dev 
*i2c_dev)
  1771  {
  1772  struct device_node *np = pdev->dev.of_node;
  1773  int ret;
  1774  u32 reg, mask;
  1775  
  1776  i2c_dev->regmap = syscon_regmap_lookup_by_phandle(np, 
"st,syscfg-fmp");
  1777  if (IS_ERR(i2c_dev->regmap)) {
  1778  /* Optional */
  1779  return 0;
  1780  }
  1781  
  1782  ret = of_property_read_u32_index(np, "st,syscfg-fmp", 1, );
  1783  if (ret)
  1784  return ret;
  1785  
  1786  ret = of_property_read_u32_index(np, "st,syscfg-fmp", 2, );
  1787  if (ret)
  1788  return ret;
  1789  
> 1790  return regmap_update_bits(i2c_dev->regmap, reg, mask, mask);
  1791  }
  1792  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v1 2/2] i2c: i2c-stm32f7: SYSCFG Fast Mode Plus support for I2C STM32F7

[kbuild test robot]

Hi Pierre-Yves,

I love your patch! Yet something to improve:

[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on v4.20-rc2 next-20181116]
[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/Pierre-Yves-MORDRET/SYSCFG-Fast-Mode-Plus-support-for-I2C-STM32F7/20181118-135056
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
i2c/for-next
config: s390-allmodconfig (attached as .config)
compiler: s390x-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=s390 

All errors (new ones prefixed by >>):

   drivers/i2c/busses/i2c-stm32f7.c: In function 
'stm32f7_i2c_setup_fm_plus_bits':
>> drivers/i2c/busses/i2c-stm32f7.c:1790:9: error: implicit declaration of 
>> function 'regmap_update_bits'; did you mean 'set_pgste_bits'? 
>> [-Werror=implicit-function-declaration]
 return regmap_update_bits(i2c_dev->regmap, reg, mask, mask);
^~
set_pgste_bits
   cc1: some warnings being treated as errors

vim +1790 drivers/i2c/busses/i2c-stm32f7.c

  1768  
  1769  static int stm32f7_i2c_setup_fm_plus_bits(struct platform_device *pdev,
  1770struct stm32f7_i2c_dev 
*i2c_dev)
  1771  {
  1772  struct device_node *np = pdev->dev.of_node;
  1773  int ret;
  1774  u32 reg, mask;
  1775  
  1776  i2c_dev->regmap = syscon_regmap_lookup_by_phandle(np, 
"st,syscfg-fmp");
  1777  if (IS_ERR(i2c_dev->regmap)) {
  1778  /* Optional */
  1779  return 0;
  1780  }
  1781  
  1782  ret = of_property_read_u32_index(np, "st,syscfg-fmp", 1, );
  1783  if (ret)
  1784  return ret;
  1785  
  1786  ret = of_property_read_u32_index(np, "st,syscfg-fmp", 2, );
  1787  if (ret)
  1788  return ret;
  1789  
> 1790  return regmap_update_bits(i2c_dev->regmap, reg, mask, mask);
  1791  }
  1792  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v1 2/2] i2c: i2c-stm32f7: SYSCFG Fast Mode Plus support for I2C STM32F7

[kbuild test robot]

Hi Pierre-Yves,

I love your patch! Yet something to improve:

[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on v4.20-rc2 next-20181116]
[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/Pierre-Yves-MORDRET/SYSCFG-Fast-Mode-Plus-support-for-I2C-STM32F7/20181118-135056
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
i2c/for-next
config: s390-allmodconfig (attached as .config)
compiler: s390x-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=s390 

All errors (new ones prefixed by >>):

   drivers/i2c/busses/i2c-stm32f7.c: In function 
'stm32f7_i2c_setup_fm_plus_bits':
>> drivers/i2c/busses/i2c-stm32f7.c:1790:9: error: implicit declaration of 
>> function 'regmap_update_bits'; did you mean 'set_pgste_bits'? 
>> [-Werror=implicit-function-declaration]
 return regmap_update_bits(i2c_dev->regmap, reg, mask, mask);
^~
set_pgste_bits
   cc1: some warnings being treated as errors

vim +1790 drivers/i2c/busses/i2c-stm32f7.c

  1768  
  1769  static int stm32f7_i2c_setup_fm_plus_bits(struct platform_device *pdev,
  1770struct stm32f7_i2c_dev 
*i2c_dev)
  1771  {
  1772  struct device_node *np = pdev->dev.of_node;
  1773  int ret;
  1774  u32 reg, mask;
  1775  
  1776  i2c_dev->regmap = syscon_regmap_lookup_by_phandle(np, 
"st,syscfg-fmp");
  1777  if (IS_ERR(i2c_dev->regmap)) {
  1778  /* Optional */
  1779  return 0;
  1780  }
  1781  
  1782  ret = of_property_read_u32_index(np, "st,syscfg-fmp", 1, );
  1783  if (ret)
  1784  return ret;
  1785  
  1786  ret = of_property_read_u32_index(np, "st,syscfg-fmp", 2, );
  1787  if (ret)
  1788  return ret;
  1789  
> 1790  return regmap_update_bits(i2c_dev->regmap, reg, mask, mask);
  1791  }
  1792  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v1 2/5] perf cs-etm: Avoid stale branch samples when flush packet

[leo . yan]

On Fri, Nov 16, 2018 at 04:05:11PM -0700, Mathieu Poirier wrote:

[...]

> > -static int cs_etm__flush(struct cs_etm_queue *etmq)
> > +static int cs_etm__flush(struct cs_etm_queue *etmq, bool new_packet)
> >  {
> > int err = 0;
> > struct cs_etm_auxtrace *etm = etmq->etm;
> > @@ -989,6 +989,20 @@ static int cs_etm__flush(struct cs_etm_queue *etmq)
> >  
> > }
> >  
> > +   /*
> > +* If 'new_packet' is false, this time call has no a new packet
> > +* coming and 'etmq->packet' contains the stale packet which is
> > +* set at the previous time with packets swapping.  In this case
> > +* this function is invoked only for flushing branch stack at
> > +* the end of buffer handling.
> > +*
> > +* Simply to say, branch samples should be generated when every
> > +* time receive one new packet; otherwise, directly bail out to
> > +* avoid generate branch sample with stale packet.
> > +*/
> > +   if (!new_packet)
> > +   return 0;
> > +
> > if (etm->sample_branches &&
> > etmq->prev_packet->sample_type == CS_ETM_RANGE) {
> > err = cs_etm__synth_branch_sample(etmq);
> > @@ -1075,7 +1089,7 @@ static int cs_etm__run_decoder(struct cs_etm_queue 
> > *etmq)
> >  * Discontinuity in trace, flush
> >  * previous branch stack
> >  */
> > -   cs_etm__flush(etmq);
> > +   cs_etm__flush(etmq, true);
> > break;
> > case CS_ETM_EMPTY:
> > /*
> > @@ -1092,7 +1106,7 @@ static int cs_etm__run_decoder(struct cs_etm_queue 
> > *etmq)
> >  
> > if (err == 0)
> > /* Flush any remaining branch stack entries */
> > -   err = cs_etm__flush(etmq);
> > +   err = cs_etm__flush(etmq, false);
> 
> I understand what you're doing and it will yield the correct results.  What 
> I'm
> not sure about is if we wouldn't be better off splitting cs_etm__flush()
> in order to reduce the complexity of the main decoding loop.  That is rename
> cs_etm__flush() to something like cs_etm__trace_on() and spin off a new
> cs_etm__end_block().  
> 
> It does introduce a little bit of code duplication but I think we'd win in 
> terms
> of readability and flexibility.

Thanks for reviewing, Mathieu.

I agree with your suggestion to split cs_etm__flush() into two
functions,  will spin this patch with the suggestion in next
series for reviewing.

Thanks,
Leo Yan

Re: [PATCH v1 2/5] perf cs-etm: Avoid stale branch samples when flush packet

[leo . yan]

On Fri, Nov 16, 2018 at 04:05:11PM -0700, Mathieu Poirier wrote:

[...]

> > -static int cs_etm__flush(struct cs_etm_queue *etmq)
> > +static int cs_etm__flush(struct cs_etm_queue *etmq, bool new_packet)
> >  {
> > int err = 0;
> > struct cs_etm_auxtrace *etm = etmq->etm;
> > @@ -989,6 +989,20 @@ static int cs_etm__flush(struct cs_etm_queue *etmq)
> >  
> > }
> >  
> > +   /*
> > +* If 'new_packet' is false, this time call has no a new packet
> > +* coming and 'etmq->packet' contains the stale packet which is
> > +* set at the previous time with packets swapping.  In this case
> > +* this function is invoked only for flushing branch stack at
> > +* the end of buffer handling.
> > +*
> > +* Simply to say, branch samples should be generated when every
> > +* time receive one new packet; otherwise, directly bail out to
> > +* avoid generate branch sample with stale packet.
> > +*/
> > +   if (!new_packet)
> > +   return 0;
> > +
> > if (etm->sample_branches &&
> > etmq->prev_packet->sample_type == CS_ETM_RANGE) {
> > err = cs_etm__synth_branch_sample(etmq);
> > @@ -1075,7 +1089,7 @@ static int cs_etm__run_decoder(struct cs_etm_queue 
> > *etmq)
> >  * Discontinuity in trace, flush
> >  * previous branch stack
> >  */
> > -   cs_etm__flush(etmq);
> > +   cs_etm__flush(etmq, true);
> > break;
> > case CS_ETM_EMPTY:
> > /*
> > @@ -1092,7 +1106,7 @@ static int cs_etm__run_decoder(struct cs_etm_queue 
> > *etmq)
> >  
> > if (err == 0)
> > /* Flush any remaining branch stack entries */
> > -   err = cs_etm__flush(etmq);
> > +   err = cs_etm__flush(etmq, false);
> 
> I understand what you're doing and it will yield the correct results.  What 
> I'm
> not sure about is if we wouldn't be better off splitting cs_etm__flush()
> in order to reduce the complexity of the main decoding loop.  That is rename
> cs_etm__flush() to something like cs_etm__trace_on() and spin off a new
> cs_etm__end_block().  
> 
> It does introduce a little bit of code duplication but I think we'd win in 
> terms
> of readability and flexibility.

Thanks for reviewing, Mathieu.

I agree with your suggestion to split cs_etm__flush() into two
functions,  will spin this patch with the suggestion in next
series for reviewing.

Thanks,
Leo Yan

Re: [PATCH 3/8] kbuild: refactor modversions build rules

[Masahiro Yamada]

Hi Sam,


On Sat, Nov 17, 2018 at 5:01 AM Sam Ravnborg  wrote:
>
> Hi Masahiro
>
> On Thu, Nov 15, 2018 at 05:27:10PM +0900, Masahiro Yamada wrote:
> > Let $(CC) compile objects into normal files *.o instead of .tmp_*.o
> > whether CONFIG_MODVERSIONS is enabled or not. This will help simplify
> > build rules a lot.
>
> Another approach would be to move more of the build stuff to
> one or a few build scripts.

Shifting code from Makefile to a shell script
will be another clean-up.


This patch is addressing a different problem.

See this code in Malefile.build

ifdef CONFIG_MODVERSIONS
objtool_o = $(@D)/.tmp_$(@F)
else
objtool_o = $(@)
endif



cmd_cc_o_c writes an object into a different file name
depending CONFIG_MODVERSIONS, which makes difficult
for other actions to manipulate the object in a
consistent manner.

I will clarify this in the commit log in v2.

Thanks.





> Using build scripts makes is much easier to add comments and
> still keep it readable.
> And when the build logic list a set of serialized actions
> they thay can be included in a build script nicely.
>
> But that said - I also like the simplifications you made.
> Everytime you trim the core kbuild files it is a win for
> readability.
>
> Sam



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 3/8] kbuild: refactor modversions build rules

[Masahiro Yamada]

Hi Sam,


On Sat, Nov 17, 2018 at 5:01 AM Sam Ravnborg  wrote:
>
> Hi Masahiro
>
> On Thu, Nov 15, 2018 at 05:27:10PM +0900, Masahiro Yamada wrote:
> > Let $(CC) compile objects into normal files *.o instead of .tmp_*.o
> > whether CONFIG_MODVERSIONS is enabled or not. This will help simplify
> > build rules a lot.
>
> Another approach would be to move more of the build stuff to
> one or a few build scripts.

Shifting code from Makefile to a shell script
will be another clean-up.


This patch is addressing a different problem.

See this code in Malefile.build

ifdef CONFIG_MODVERSIONS
objtool_o = $(@D)/.tmp_$(@F)
else
objtool_o = $(@)
endif



cmd_cc_o_c writes an object into a different file name
depending CONFIG_MODVERSIONS, which makes difficult
for other actions to manipulate the object in a
consistent manner.

I will clarify this in the commit log in v2.

Thanks.





> Using build scripts makes is much easier to add comments and
> still keep it readable.
> And when the build logic list a set of serialized actions
> they thay can be included in a build script nicely.
>
> But that said - I also like the simplifications you made.
> Everytime you trim the core kbuild files it is a win for
> readability.
>
> Sam



-- 
Best Regards
Masahiro Yamada

[PATCH v2 2/7] staging:iio:ad2s90: Remove spi setup that should be done via dt

[Matheus Tavares]

The ad2s90 driver currently sets some spi settings (max_speed_hz and
mode) at ad2s90_probe. Since the maximum frequency is a required element
in DT binding for spi slave devices and because the spi mode for the
device can be either (0,0) or (1,1), these settings should be handled
via device tree, not in the driver's code. This patch removes them from
the probe function.

Note: The way in which the mentioned spi settings need to be specified
on the ad2s90's node of a device tree will be documented in the future
patch "dt-bindings:iio:resolver: Add docs for ad2s90".

Signed-off-by: Matheus Tavares 
---
Changes in v2:
 - Rewritten patch message to better explain why the code snippet in
 question should be removed.

 drivers/staging/iio/resolver/ad2s90.c | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s90.c 
b/drivers/staging/iio/resolver/ad2s90.c
index 6ffbac66b837..913d6fad2d4d 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -77,7 +77,6 @@ static int ad2s90_probe(struct spi_device *spi)
 {
struct iio_dev *indio_dev;
struct ad2s90_state *st;
-   int ret;
 
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
@@ -94,16 +93,6 @@ static int ad2s90_probe(struct spi_device *spi)
indio_dev->num_channels = 1;
indio_dev->name = spi_get_device_id(spi)->name;
 
-   /* need 600ns between CS and the first falling edge of SCLK */
-   spi->max_speed_hz = 83;
-   spi->mode = SPI_MODE_3;
-   ret = spi_setup(spi);
-
-   if (ret < 0) {
-   dev_err(>dev, "spi_setup failed!\n");
-   return ret;
-   }
-
return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
 }
 
-- 
2.18.0

[PATCH v2 6/7] staging:iio:ad2s90: Add comment to device state mutex

[Matheus Tavares]

From: Victor Colombo 

Fix the checkpatch.pl issue:
"CHECK: struct mutex definition without comment".

Signed-off-by: Victor Colombo 
Signed-off-by: Matheus Tavares 
---
Changes in v2:
 - Patch added in v2

 drivers/staging/iio/resolver/ad2s90.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/resolver/ad2s90.c 
b/drivers/staging/iio/resolver/ad2s90.c
index 9aa229ba47e7..f04dc5dede00 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -22,7 +22,7 @@
 #define AD2S90_MAX_SPI_FREQ_HZ  83
 
 struct ad2s90_state {
-   struct mutex lock;
+   struct mutex lock; /* lock to protect rx buffer */
struct spi_device *sdev;
u8 rx[2] cacheline_aligned;
 };
-- 
2.18.0

[PATCH v2 4/7] dt-bindings:iio:resolver: Add docs for ad2s90

[Matheus Tavares]

This patch adds the device tree binding documentation for the ad2s90
resolver-to-digital converter.

Signed-off-by: Matheus Tavares 
---
Changes in v2:
 - Rewritten 'spi-cpol and spi-cpha' item to say that the device can
 work in either mode (0,0) or (1,1) and explain how they should be
 specified in DT.

 .../bindings/iio/resolver/ad2s90.txt  | 28 +++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/resolver/ad2s90.txt

diff --git a/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt 
b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
new file mode 100644
index ..594417539938
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
@@ -0,0 +1,28 @@
+Analog Devices AD2S90 Resolver-to-Digital Converter
+
+https://www.analog.com/en/products/ad2s90.html
+
+Required properties:
+  - compatible: should be "adi,ad2s90"
+  - reg: SPI chip select number for the device
+  - spi-max-frequency: set maximum clock frequency, must be 83
+  - spi-cpol and spi-cpha:
+Either SPI mode (0,0) or (1,1) must be used, so specify none or both of
+spi-cpha, spi-cpol.
+
+Note about max frequency:
+Chip's max frequency, as specified in its datasheet, is 2Mhz. But a 600ns
+delay is expected between the application of a logic LO to CS and the
+application of SCLK, as also specified. And since the delay is not
+implemented in the spi code, to satisfy it, SCLK's period should be at most
+2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which gives
+roughly 83Hz.
+
+Example:
+resolver@0 {
+   compatible = "adi,ad2s90";
+   reg = <0>;
+   spi-max-frequency = <83>;
+   spi-cpol;
+   spi-cpha;
+};
-- 
2.18.0

[PATCH v2 2/7] staging:iio:ad2s90: Remove spi setup that should be done via dt

[Matheus Tavares]

The ad2s90 driver currently sets some spi settings (max_speed_hz and
mode) at ad2s90_probe. Since the maximum frequency is a required element
in DT binding for spi slave devices and because the spi mode for the
device can be either (0,0) or (1,1), these settings should be handled
via device tree, not in the driver's code. This patch removes them from
the probe function.

Note: The way in which the mentioned spi settings need to be specified
on the ad2s90's node of a device tree will be documented in the future
patch "dt-bindings:iio:resolver: Add docs for ad2s90".

Signed-off-by: Matheus Tavares 
---
Changes in v2:
 - Rewritten patch message to better explain why the code snippet in
 question should be removed.

 drivers/staging/iio/resolver/ad2s90.c | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s90.c 
b/drivers/staging/iio/resolver/ad2s90.c
index 6ffbac66b837..913d6fad2d4d 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -77,7 +77,6 @@ static int ad2s90_probe(struct spi_device *spi)
 {
struct iio_dev *indio_dev;
struct ad2s90_state *st;
-   int ret;
 
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
@@ -94,16 +93,6 @@ static int ad2s90_probe(struct spi_device *spi)
indio_dev->num_channels = 1;
indio_dev->name = spi_get_device_id(spi)->name;
 
-   /* need 600ns between CS and the first falling edge of SCLK */
-   spi->max_speed_hz = 83;
-   spi->mode = SPI_MODE_3;
-   ret = spi_setup(spi);
-
-   if (ret < 0) {
-   dev_err(>dev, "spi_setup failed!\n");
-   return ret;
-   }
-
return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
 }
 
-- 
2.18.0

[PATCH v2 6/7] staging:iio:ad2s90: Add comment to device state mutex

[Matheus Tavares]

From: Victor Colombo 

Fix the checkpatch.pl issue:
"CHECK: struct mutex definition without comment".

Signed-off-by: Victor Colombo 
Signed-off-by: Matheus Tavares 
---
Changes in v2:
 - Patch added in v2

 drivers/staging/iio/resolver/ad2s90.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/resolver/ad2s90.c 
b/drivers/staging/iio/resolver/ad2s90.c
index 9aa229ba47e7..f04dc5dede00 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -22,7 +22,7 @@
 #define AD2S90_MAX_SPI_FREQ_HZ  83
 
 struct ad2s90_state {
-   struct mutex lock;
+   struct mutex lock; /* lock to protect rx buffer */
struct spi_device *sdev;
u8 rx[2] cacheline_aligned;
 };
-- 
2.18.0

[PATCH v2 4/7] dt-bindings:iio:resolver: Add docs for ad2s90

[Matheus Tavares]

This patch adds the device tree binding documentation for the ad2s90
resolver-to-digital converter.

Signed-off-by: Matheus Tavares 
---
Changes in v2:
 - Rewritten 'spi-cpol and spi-cpha' item to say that the device can
 work in either mode (0,0) or (1,1) and explain how they should be
 specified in DT.

 .../bindings/iio/resolver/ad2s90.txt  | 28 +++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/resolver/ad2s90.txt

diff --git a/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt 
b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
new file mode 100644
index ..594417539938
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
@@ -0,0 +1,28 @@
+Analog Devices AD2S90 Resolver-to-Digital Converter
+
+https://www.analog.com/en/products/ad2s90.html
+
+Required properties:
+  - compatible: should be "adi,ad2s90"
+  - reg: SPI chip select number for the device
+  - spi-max-frequency: set maximum clock frequency, must be 83
+  - spi-cpol and spi-cpha:
+Either SPI mode (0,0) or (1,1) must be used, so specify none or both of
+spi-cpha, spi-cpol.
+
+Note about max frequency:
+Chip's max frequency, as specified in its datasheet, is 2Mhz. But a 600ns
+delay is expected between the application of a logic LO to CS and the
+application of SCLK, as also specified. And since the delay is not
+implemented in the spi code, to satisfy it, SCLK's period should be at most
+2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which gives
+roughly 83Hz.
+
+Example:
+resolver@0 {
+   compatible = "adi,ad2s90";
+   reg = <0>;
+   spi-max-frequency = <83>;
+   spi-cpol;
+   spi-cpha;
+};
-- 
2.18.0

[PATCH v2 5/7] staging:iio:ad2s90: Replace license text w/ SPDX identifier

[Matheus Tavares]

This patch removes the license boilerplate text at the top of ad2s90.c
and, instead, adds the SPDX GPL-2.0 license identifier, which solves the
checkpatch.pl warning:
"WARNING: Missing or malformed SPDX-License-Identifier tag in line 1".

Signed-off-by: Matheus Tavares 
---
Changes in v2:
 - Changed GPL-2.0-only identifier to GPL-2.0
 - Removed license boilerplate text
 - Rewritten patch message to reflect these modifications

 drivers/staging/iio/resolver/ad2s90.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s90.c 
b/drivers/staging/iio/resolver/ad2s90.c
index fe90f2056bff..9aa229ba47e7 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -1,12 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * ad2s90.c simple support for the ADI Resolver to Digital Converters: AD2S90
  *
  * Copyright (c) 2010-2010 Analog Devices Inc.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
  */
 #include 
 #include 
-- 
2.18.0

[PATCH v2 7/7] staging:iio:ad2s90: Move out of staging

[Matheus Tavares]

Move ad2s90 resolver driver out of staging to the main tree.

Signed-off-by: Matheus Tavares 
Signed-off-by: Victor Colombo 
---
Changes in v2:
 - Disabled git move detection, to see the whole code, as Jonathan
 suggested

 drivers/iio/resolver/Kconfig  |  10 ++
 drivers/iio/resolver/Makefile |   1 +
 drivers/iio/resolver/ad2s90.c | 131 ++
 drivers/staging/iio/resolver/Kconfig  |  10 --
 drivers/staging/iio/resolver/Makefile |   1 -
 drivers/staging/iio/resolver/ad2s90.c | 131 --
 6 files changed, 142 insertions(+), 142 deletions(-)
 create mode 100644 drivers/iio/resolver/ad2s90.c
 delete mode 100644 drivers/staging/iio/resolver/ad2s90.c

diff --git a/drivers/iio/resolver/Kconfig b/drivers/iio/resolver/Kconfig
index 2ced9f22aa70..786801be54f6 100644
--- a/drivers/iio/resolver/Kconfig
+++ b/drivers/iio/resolver/Kconfig
@@ -3,6 +3,16 @@
 #
 menu "Resolver to digital converters"
 
+config AD2S90
+   tristate "Analog Devices ad2s90 driver"
+   depends on SPI
+   help
+ Say yes here to build support for Analog Devices spi resolver
+ to digital converters, ad2s90, provides direct access via sysfs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ad2s90.
+
 config AD2S1200
tristate "Analog Devices ad2s1200/ad2s1205 driver"
depends on SPI
diff --git a/drivers/iio/resolver/Makefile b/drivers/iio/resolver/Makefile
index 4e1dccae07e7..398d82d50028 100644
--- a/drivers/iio/resolver/Makefile
+++ b/drivers/iio/resolver/Makefile
@@ -2,4 +2,5 @@
 # Makefile for Resolver/Synchro drivers
 #
 
+obj-$(CONFIG_AD2S90) += ad2s90.o
 obj-$(CONFIG_AD2S1200) += ad2s1200.o
diff --git a/drivers/iio/resolver/ad2s90.c b/drivers/iio/resolver/ad2s90.c
new file mode 100644
index ..f04dc5dede00
--- /dev/null
+++ b/drivers/iio/resolver/ad2s90.c
@@ -0,0 +1,131 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ad2s90.c simple support for the ADI Resolver to Digital Converters: AD2S90
+ *
+ * Copyright (c) 2010-2010 Analog Devices Inc.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+/*
+ * Although chip's max frequency is 2Mhz, it needs 600ns between CS and the
+ * first falling edge of SCLK, so frequency should be at most 1 / (2 * 6e-7)
+ */
+#define AD2S90_MAX_SPI_FREQ_HZ  83
+
+struct ad2s90_state {
+   struct mutex lock; /* lock to protect rx buffer */
+   struct spi_device *sdev;
+   u8 rx[2] cacheline_aligned;
+};
+
+static int ad2s90_read_raw(struct iio_dev *indio_dev,
+  struct iio_chan_spec const *chan,
+  int *val,
+  int *val2,
+  long m)
+{
+   int ret;
+   struct ad2s90_state *st = iio_priv(indio_dev);
+
+   if (chan->type != IIO_ANGL)
+   return -EINVAL;
+
+   switch (m) {
+   case IIO_CHAN_INFO_SCALE:
+   /* 2 * Pi / 2^12 */
+   *val = 6283; /* mV */
+   *val2 = 12;
+   return IIO_VAL_FRACTIONAL_LOG2;
+   case IIO_CHAN_INFO_RAW:
+   mutex_lock(>lock);
+   ret = spi_read(st->sdev, st->rx, 2);
+   if (ret < 0) {
+   mutex_unlock(>lock);
+   return ret;
+   }
+   *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
+
+   mutex_unlock(>lock);
+
+   return IIO_VAL_INT;
+   default:
+   break;
+   }
+
+   return -EINVAL;
+}
+
+static const struct iio_info ad2s90_info = {
+   .read_raw = ad2s90_read_raw,
+};
+
+static const struct iio_chan_spec ad2s90_chan = {
+   .type = IIO_ANGL,
+   .indexed = 1,
+   .channel = 0,
+   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+};
+
+static int ad2s90_probe(struct spi_device *spi)
+{
+   struct iio_dev *indio_dev;
+   struct ad2s90_state *st;
+
+   if (spi->max_speed_hz > AD2S90_MAX_SPI_FREQ_HZ) {
+   dev_err(>dev, "SPI CLK, %d Hz exceeds %d Hz\n",
+   spi->max_speed_hz, AD2S90_MAX_SPI_FREQ_HZ);
+   return -EINVAL;
+   }
+
+   indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
+   if (!indio_dev)
+   return -ENOMEM;
+   st = iio_priv(indio_dev);
+   spi_set_drvdata(spi, indio_dev);
+
+   mutex_init(>lock);
+   st->sdev = spi;
+   indio_dev->dev.parent = >dev;
+   indio_dev->info = _info;
+   indio_dev->modes = INDIO_DIRECT_MODE;
+   indio_dev->channels = _chan;
+   indio_dev->num_channels = 1;
+   indio_dev->name = spi_get_device_id(spi)->name;
+
+   return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
+}
+
+static const struct of_device_id ad2s90_of_match[] = {
+   { .compatible = "adi,ad2s90", },
+   {}
+};

[PATCH v2 1/7] staging:iio:ad2s90: Add device tree support

[Matheus Tavares]

This patch adds device tree support to ad2s90 with standard
device tree id table.

Signed-off-by: Matheus Tavares 
---
Changes in v2:
 - none

 drivers/staging/iio/resolver/ad2s90.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/staging/iio/resolver/ad2s90.c 
b/drivers/staging/iio/resolver/ad2s90.c
index 3e257ac46f7a..6ffbac66b837 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -107,6 +107,12 @@ static int ad2s90_probe(struct spi_device *spi)
return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
 }
 
+static const struct of_device_id ad2s90_of_match[] = {
+   { .compatible = "adi,ad2s90", },
+   {}
+};
+MODULE_DEVICE_TABLE(of, ad2s90_of_match);
+
 static const struct spi_device_id ad2s90_id[] = {
{ "ad2s90" },
{}
@@ -116,6 +122,7 @@ MODULE_DEVICE_TABLE(spi, ad2s90_id);
 static struct spi_driver ad2s90_driver = {
.driver = {
.name = "ad2s90",
+   .of_match_table = of_match_ptr(ad2s90_of_match),
},
.probe = ad2s90_probe,
.id_table = ad2s90_id,
-- 
2.18.0

[PATCH v2 3/7] staging:iio:ad2s90: Add max frequency check at probe

[Matheus Tavares]

From: Alexandru Ardelean 

This patch adds a max frequency check at the beginning of ad2s90_probe
function so that when it is set to a value above 0.83Mhz, dev_err is
called with an appropriate message and -EINVAL is returned.

The defined limit is 0.83Mhz instead of 2Mhz, which is the chip's max
frequency as specified in the datasheet, because, as also specified in
the datasheet, a 600ns delay is expected between the application of a
logic LO to CS and the application of SCLK. Since the delay is not
implemented in the spi code, to satisfy it, SCLK's period should be at
most 2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which
gives roughly 83Hz.

Signed-off-by: Alexandru Ardelean 
Signed-off-by: Matheus Tavares 
---
Changes in v2:
 - Correctly credit Alexandru as the patch's author

 drivers/staging/iio/resolver/ad2s90.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/staging/iio/resolver/ad2s90.c 
b/drivers/staging/iio/resolver/ad2s90.c
index 913d6fad2d4d..fe90f2056bff 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -19,6 +19,12 @@
 #include 
 #include 
 
+/*
+ * Although chip's max frequency is 2Mhz, it needs 600ns between CS and the
+ * first falling edge of SCLK, so frequency should be at most 1 / (2 * 6e-7)
+ */
+#define AD2S90_MAX_SPI_FREQ_HZ  83
+
 struct ad2s90_state {
struct mutex lock;
struct spi_device *sdev;
@@ -78,6 +84,12 @@ static int ad2s90_probe(struct spi_device *spi)
struct iio_dev *indio_dev;
struct ad2s90_state *st;
 
+   if (spi->max_speed_hz > AD2S90_MAX_SPI_FREQ_HZ) {
+   dev_err(>dev, "SPI CLK, %d Hz exceeds %d Hz\n",
+   spi->max_speed_hz, AD2S90_MAX_SPI_FREQ_HZ);
+   return -EINVAL;
+   }
+
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
return -ENOMEM;
-- 
2.18.0

[PATCH v2 5/7] staging:iio:ad2s90: Replace license text w/ SPDX identifier

[Matheus Tavares]

This patch removes the license boilerplate text at the top of ad2s90.c
and, instead, adds the SPDX GPL-2.0 license identifier, which solves the
checkpatch.pl warning:
"WARNING: Missing or malformed SPDX-License-Identifier tag in line 1".

Signed-off-by: Matheus Tavares 
---
Changes in v2:
 - Changed GPL-2.0-only identifier to GPL-2.0
 - Removed license boilerplate text
 - Rewritten patch message to reflect these modifications

 drivers/staging/iio/resolver/ad2s90.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s90.c 
b/drivers/staging/iio/resolver/ad2s90.c
index fe90f2056bff..9aa229ba47e7 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -1,12 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * ad2s90.c simple support for the ADI Resolver to Digital Converters: AD2S90
  *
  * Copyright (c) 2010-2010 Analog Devices Inc.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
  */
 #include 
 #include 
-- 
2.18.0

[PATCH v2 7/7] staging:iio:ad2s90: Move out of staging

[Matheus Tavares]

Move ad2s90 resolver driver out of staging to the main tree.

Signed-off-by: Matheus Tavares 
Signed-off-by: Victor Colombo 
---
Changes in v2:
 - Disabled git move detection, to see the whole code, as Jonathan
 suggested

 drivers/iio/resolver/Kconfig  |  10 ++
 drivers/iio/resolver/Makefile |   1 +
 drivers/iio/resolver/ad2s90.c | 131 ++
 drivers/staging/iio/resolver/Kconfig  |  10 --
 drivers/staging/iio/resolver/Makefile |   1 -
 drivers/staging/iio/resolver/ad2s90.c | 131 --
 6 files changed, 142 insertions(+), 142 deletions(-)
 create mode 100644 drivers/iio/resolver/ad2s90.c
 delete mode 100644 drivers/staging/iio/resolver/ad2s90.c

diff --git a/drivers/iio/resolver/Kconfig b/drivers/iio/resolver/Kconfig
index 2ced9f22aa70..786801be54f6 100644
--- a/drivers/iio/resolver/Kconfig
+++ b/drivers/iio/resolver/Kconfig
@@ -3,6 +3,16 @@
 #
 menu "Resolver to digital converters"
 
+config AD2S90
+   tristate "Analog Devices ad2s90 driver"
+   depends on SPI
+   help
+ Say yes here to build support for Analog Devices spi resolver
+ to digital converters, ad2s90, provides direct access via sysfs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ad2s90.
+
 config AD2S1200
tristate "Analog Devices ad2s1200/ad2s1205 driver"
depends on SPI
diff --git a/drivers/iio/resolver/Makefile b/drivers/iio/resolver/Makefile
index 4e1dccae07e7..398d82d50028 100644
--- a/drivers/iio/resolver/Makefile
+++ b/drivers/iio/resolver/Makefile
@@ -2,4 +2,5 @@
 # Makefile for Resolver/Synchro drivers
 #
 
+obj-$(CONFIG_AD2S90) += ad2s90.o
 obj-$(CONFIG_AD2S1200) += ad2s1200.o
diff --git a/drivers/iio/resolver/ad2s90.c b/drivers/iio/resolver/ad2s90.c
new file mode 100644
index ..f04dc5dede00
--- /dev/null
+++ b/drivers/iio/resolver/ad2s90.c
@@ -0,0 +1,131 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ad2s90.c simple support for the ADI Resolver to Digital Converters: AD2S90
+ *
+ * Copyright (c) 2010-2010 Analog Devices Inc.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+/*
+ * Although chip's max frequency is 2Mhz, it needs 600ns between CS and the
+ * first falling edge of SCLK, so frequency should be at most 1 / (2 * 6e-7)
+ */
+#define AD2S90_MAX_SPI_FREQ_HZ  83
+
+struct ad2s90_state {
+   struct mutex lock; /* lock to protect rx buffer */
+   struct spi_device *sdev;
+   u8 rx[2] cacheline_aligned;
+};
+
+static int ad2s90_read_raw(struct iio_dev *indio_dev,
+  struct iio_chan_spec const *chan,
+  int *val,
+  int *val2,
+  long m)
+{
+   int ret;
+   struct ad2s90_state *st = iio_priv(indio_dev);
+
+   if (chan->type != IIO_ANGL)
+   return -EINVAL;
+
+   switch (m) {
+   case IIO_CHAN_INFO_SCALE:
+   /* 2 * Pi / 2^12 */
+   *val = 6283; /* mV */
+   *val2 = 12;
+   return IIO_VAL_FRACTIONAL_LOG2;
+   case IIO_CHAN_INFO_RAW:
+   mutex_lock(>lock);
+   ret = spi_read(st->sdev, st->rx, 2);
+   if (ret < 0) {
+   mutex_unlock(>lock);
+   return ret;
+   }
+   *val = (((u16)(st->rx[0])) << 4) | ((st->rx[1] & 0xF0) >> 4);
+
+   mutex_unlock(>lock);
+
+   return IIO_VAL_INT;
+   default:
+   break;
+   }
+
+   return -EINVAL;
+}
+
+static const struct iio_info ad2s90_info = {
+   .read_raw = ad2s90_read_raw,
+};
+
+static const struct iio_chan_spec ad2s90_chan = {
+   .type = IIO_ANGL,
+   .indexed = 1,
+   .channel = 0,
+   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+};
+
+static int ad2s90_probe(struct spi_device *spi)
+{
+   struct iio_dev *indio_dev;
+   struct ad2s90_state *st;
+
+   if (spi->max_speed_hz > AD2S90_MAX_SPI_FREQ_HZ) {
+   dev_err(>dev, "SPI CLK, %d Hz exceeds %d Hz\n",
+   spi->max_speed_hz, AD2S90_MAX_SPI_FREQ_HZ);
+   return -EINVAL;
+   }
+
+   indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
+   if (!indio_dev)
+   return -ENOMEM;
+   st = iio_priv(indio_dev);
+   spi_set_drvdata(spi, indio_dev);
+
+   mutex_init(>lock);
+   st->sdev = spi;
+   indio_dev->dev.parent = >dev;
+   indio_dev->info = _info;
+   indio_dev->modes = INDIO_DIRECT_MODE;
+   indio_dev->channels = _chan;
+   indio_dev->num_channels = 1;
+   indio_dev->name = spi_get_device_id(spi)->name;
+
+   return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
+}
+
+static const struct of_device_id ad2s90_of_match[] = {
+   { .compatible = "adi,ad2s90", },
+   {}
+};

[PATCH v2 1/7] staging:iio:ad2s90: Add device tree support

[Matheus Tavares]

This patch adds device tree support to ad2s90 with standard
device tree id table.

Signed-off-by: Matheus Tavares 
---
Changes in v2:
 - none

 drivers/staging/iio/resolver/ad2s90.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/staging/iio/resolver/ad2s90.c 
b/drivers/staging/iio/resolver/ad2s90.c
index 3e257ac46f7a..6ffbac66b837 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -107,6 +107,12 @@ static int ad2s90_probe(struct spi_device *spi)
return devm_iio_device_register(indio_dev->dev.parent, indio_dev);
 }
 
+static const struct of_device_id ad2s90_of_match[] = {
+   { .compatible = "adi,ad2s90", },
+   {}
+};
+MODULE_DEVICE_TABLE(of, ad2s90_of_match);
+
 static const struct spi_device_id ad2s90_id[] = {
{ "ad2s90" },
{}
@@ -116,6 +122,7 @@ MODULE_DEVICE_TABLE(spi, ad2s90_id);
 static struct spi_driver ad2s90_driver = {
.driver = {
.name = "ad2s90",
+   .of_match_table = of_match_ptr(ad2s90_of_match),
},
.probe = ad2s90_probe,
.id_table = ad2s90_id,
-- 
2.18.0

[PATCH v2 3/7] staging:iio:ad2s90: Add max frequency check at probe

[Matheus Tavares]

From: Alexandru Ardelean 

This patch adds a max frequency check at the beginning of ad2s90_probe
function so that when it is set to a value above 0.83Mhz, dev_err is
called with an appropriate message and -EINVAL is returned.

The defined limit is 0.83Mhz instead of 2Mhz, which is the chip's max
frequency as specified in the datasheet, because, as also specified in
the datasheet, a 600ns delay is expected between the application of a
logic LO to CS and the application of SCLK. Since the delay is not
implemented in the spi code, to satisfy it, SCLK's period should be at
most 2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which
gives roughly 83Hz.

Signed-off-by: Alexandru Ardelean 
Signed-off-by: Matheus Tavares 
---
Changes in v2:
 - Correctly credit Alexandru as the patch's author

 drivers/staging/iio/resolver/ad2s90.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/staging/iio/resolver/ad2s90.c 
b/drivers/staging/iio/resolver/ad2s90.c
index 913d6fad2d4d..fe90f2056bff 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -19,6 +19,12 @@
 #include 
 #include 
 
+/*
+ * Although chip's max frequency is 2Mhz, it needs 600ns between CS and the
+ * first falling edge of SCLK, so frequency should be at most 1 / (2 * 6e-7)
+ */
+#define AD2S90_MAX_SPI_FREQ_HZ  83
+
 struct ad2s90_state {
struct mutex lock;
struct spi_device *sdev;
@@ -78,6 +84,12 @@ static int ad2s90_probe(struct spi_device *spi)
struct iio_dev *indio_dev;
struct ad2s90_state *st;
 
+   if (spi->max_speed_hz > AD2S90_MAX_SPI_FREQ_HZ) {
+   dev_err(>dev, "SPI CLK, %d Hz exceeds %d Hz\n",
+   spi->max_speed_hz, AD2S90_MAX_SPI_FREQ_HZ);
+   return -EINVAL;
+   }
+
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
return -ENOMEM;
-- 
2.18.0

[PATCH v2 0/7] staging:iio:ad2s90: Add dt support and move out of staging

[Matheus Tavares]

This series adds device tree support to ad2s90, adds the respective dt-binding  
documentation, solves all remaining codestyle problems in the driver code and
move it out of staging. 


This patch set completes all the remaining itens listed to be done before moving
the driver out of staging, enumerated in this mail thread:  
https://marc.info/?l=linux-iio=154028966111330=2.

Alexandru Ardelean (1):
  staging:iio:ad2s90: Add max frequency check at probe

Matheus Tavares (5):
  staging:iio:ad2s90: Add device tree support
  staging:iio:ad2s90: Remove spi setup that should be done via dt
  dt-bindings:iio:resolver: Add docs for ad2s90
  staging:iio:ad2s90: Replace license text w/ SPDX identifier
  staging:iio:ad2s90: Move out of staging

Victor Colombo (1):
  staging:iio:ad2s90: Add comment to device state mutex

 .../bindings/iio/resolver/ad2s90.txt  |  28 
 drivers/iio/resolver/Kconfig  |  10 ++
 drivers/iio/resolver/Makefile |   1 +
 drivers/iio/resolver/ad2s90.c | 131 ++
 drivers/staging/iio/resolver/Kconfig  |  10 --
 drivers/staging/iio/resolver/Makefile |   1 -
 drivers/staging/iio/resolver/ad2s90.c | 127 -
 7 files changed, 170 insertions(+), 138 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
 create mode 100644 drivers/iio/resolver/ad2s90.c
 delete mode 100644 drivers/staging/iio/resolver/ad2s90.c

-- 
2.18.0

[PATCH v2 0/7] staging:iio:ad2s90: Add dt support and move out of staging

[Matheus Tavares]

This series adds device tree support to ad2s90, adds the respective dt-binding  
documentation, solves all remaining codestyle problems in the driver code and
move it out of staging. 


This patch set completes all the remaining itens listed to be done before moving
the driver out of staging, enumerated in this mail thread:  
https://marc.info/?l=linux-iio=154028966111330=2.

Alexandru Ardelean (1):
  staging:iio:ad2s90: Add max frequency check at probe

Matheus Tavares (5):
  staging:iio:ad2s90: Add device tree support
  staging:iio:ad2s90: Remove spi setup that should be done via dt
  dt-bindings:iio:resolver: Add docs for ad2s90
  staging:iio:ad2s90: Replace license text w/ SPDX identifier
  staging:iio:ad2s90: Move out of staging

Victor Colombo (1):
  staging:iio:ad2s90: Add comment to device state mutex

 .../bindings/iio/resolver/ad2s90.txt  |  28 
 drivers/iio/resolver/Kconfig  |  10 ++
 drivers/iio/resolver/Makefile |   1 +
 drivers/iio/resolver/ad2s90.c | 131 ++
 drivers/staging/iio/resolver/Kconfig  |  10 --
 drivers/staging/iio/resolver/Makefile |   1 -
 drivers/staging/iio/resolver/ad2s90.c | 127 -
 7 files changed, 170 insertions(+), 138 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/resolver/ad2s90.txt
 create mode 100644 drivers/iio/resolver/ad2s90.c
 delete mode 100644 drivers/staging/iio/resolver/ad2s90.c

-- 
2.18.0

Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros

[Masahiro Yamada]

On Sat, Nov 17, 2018 at 6:02 AM Nadav Amit  wrote:
>
> From: Masahiro Yamada
> Sent: November 16, 2018 at 7:45:45 AM GMT
> > To: Nadav Amit 
> > Cc: Ingo Molnar , Michal Marek , 
> > Thomas Gleixner , Borislav Petkov , H. 
> > Peter Anvin , X86 ML , Linux Kbuild 
> > mailing list , Linux Kernel Mailing List 
> > 
> > Subject: Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros
> >
> >
> > On Thu, Nov 15, 2018 at 1:01 PM Nadav Amit  wrote:
> >> Introducing the use of asm macros in c-code broke distcc, since it only
> >> sends the preprocessed source file. The solution is to break the
> >> compilation into two separate phases of compilation and assembly, and
> >> between the two concatenate the assembly macros and the compiled (yet
> >> not assembled) source file. Since this is less efficient, this
> >> compilation mode is only used when distcc or icecc are used.
> >>
> >> Note that the assembly stage should also be distributed, if distcc is
> >> configured using "CFLAGS=-DENABLE_REMOTE_ASSEMBLE".
> >>
> >> Reported-by: Logan Gunthorpe 
> >> Signed-off-by: Nadav Amit 
> >
> >
> > Wow, this is so ugly.
>
> Indeed, it is not pretty from the Makefile system point of view.
>
> But it does have merits when it comes to the actual use (by developers). If
> you look on x86, there are a lot of duplicated implementation for C and Asm
> and crazy C macros, for example for PV function call or the alternative
> mechanism. The code is also less readable in its current form.
>
> > I realized how much I hated this by now.
> >
> > My question is, how long do we need to carry this?
>
> If it is purely about performance, I am not sure it would make sense to
> put it in, as it will indeed be (eventually) solved by the compiler, and
> the penalty is not too great.


It is unfortunate to mess up the code
by having two different ways to achieve the same goal.

When GCC with asm inline support is shipped,
would you revert 77b0bf55 ?



> There is also an advantage for having assembly macros that can override the
> generated assembly that was generated by the compiler. I think HPA (cc’d)
> looks in this direction (and so do I).
>
> Regards,
> Nadav



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros

[Masahiro Yamada]

On Sat, Nov 17, 2018 at 6:02 AM Nadav Amit  wrote:
>
> From: Masahiro Yamada
> Sent: November 16, 2018 at 7:45:45 AM GMT
> > To: Nadav Amit 
> > Cc: Ingo Molnar , Michal Marek , 
> > Thomas Gleixner , Borislav Petkov , H. 
> > Peter Anvin , X86 ML , Linux Kbuild 
> > mailing list , Linux Kernel Mailing List 
> > 
> > Subject: Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros
> >
> >
> > On Thu, Nov 15, 2018 at 1:01 PM Nadav Amit  wrote:
> >> Introducing the use of asm macros in c-code broke distcc, since it only
> >> sends the preprocessed source file. The solution is to break the
> >> compilation into two separate phases of compilation and assembly, and
> >> between the two concatenate the assembly macros and the compiled (yet
> >> not assembled) source file. Since this is less efficient, this
> >> compilation mode is only used when distcc or icecc are used.
> >>
> >> Note that the assembly stage should also be distributed, if distcc is
> >> configured using "CFLAGS=-DENABLE_REMOTE_ASSEMBLE".
> >>
> >> Reported-by: Logan Gunthorpe 
> >> Signed-off-by: Nadav Amit 
> >
> >
> > Wow, this is so ugly.
>
> Indeed, it is not pretty from the Makefile system point of view.
>
> But it does have merits when it comes to the actual use (by developers). If
> you look on x86, there are a lot of duplicated implementation for C and Asm
> and crazy C macros, for example for PV function call or the alternative
> mechanism. The code is also less readable in its current form.
>
> > I realized how much I hated this by now.
> >
> > My question is, how long do we need to carry this?
>
> If it is purely about performance, I am not sure it would make sense to
> put it in, as it will indeed be (eventually) solved by the compiler, and
> the penalty is not too great.


It is unfortunate to mess up the code
by having two different ways to achieve the same goal.

When GCC with asm inline support is shipped,
would you revert 77b0bf55 ?



> There is also an advantage for having assembly macros that can override the
> generated assembly that was generated by the compiler. I think HPA (cc’d)
> looks in this direction (and so do I).
>
> Regards,
> Nadav



-- 
Best Regards
Masahiro Yamada

[PATCH] clk: rockchip: fix ID of 8ch clock of I2S1 for rk3328

[Katsuhiro Suzuki]

This patch fixes mistakes in HCLK_I2S1_8CH for running I2S1
successfully.

Signed-off-by: Katsuhiro Suzuki 
---
 include/dt-bindings/clock/rk3328-cru.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/dt-bindings/clock/rk3328-cru.h 
b/include/dt-bindings/clock/rk3328-cru.h
index a82a0109faff..bcaa4559ab1b 100644
--- a/include/dt-bindings/clock/rk3328-cru.h
+++ b/include/dt-bindings/clock/rk3328-cru.h
@@ -172,13 +172,14 @@
 #define PCLK_HDCP  232
 #define PCLK_DCF   233
 #define PCLK_SARADC234
+#define PCLK_ACODECPHY 235
 
 /* hclk gates */
 #define HCLK_PERI  308
 #define HCLK_TSP   309
 #define HCLK_GMAC  310
 #define HCLK_I2S0_8CH  311
-#define HCLK_I2S1_8CH  313
+#define HCLK_I2S1_8CH  312
 #define HCLK_I2S2_2CH  313
 #define HCLK_SPDIF_8CH 314
 #define HCLK_VOP   315
-- 
2.19.1

[PATCH] clk: rockchip: fix ID of 8ch clock of I2S1 for rk3328

[Katsuhiro Suzuki]

This patch fixes mistakes in HCLK_I2S1_8CH for running I2S1
successfully.

Signed-off-by: Katsuhiro Suzuki 
---
 include/dt-bindings/clock/rk3328-cru.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/dt-bindings/clock/rk3328-cru.h 
b/include/dt-bindings/clock/rk3328-cru.h
index a82a0109faff..bcaa4559ab1b 100644
--- a/include/dt-bindings/clock/rk3328-cru.h
+++ b/include/dt-bindings/clock/rk3328-cru.h
@@ -172,13 +172,14 @@
 #define PCLK_HDCP  232
 #define PCLK_DCF   233
 #define PCLK_SARADC234
+#define PCLK_ACODECPHY 235
 
 /* hclk gates */
 #define HCLK_PERI  308
 #define HCLK_TSP   309
 #define HCLK_GMAC  310
 #define HCLK_I2S0_8CH  311
-#define HCLK_I2S1_8CH  313
+#define HCLK_I2S1_8CH  312
 #define HCLK_I2S2_2CH  313
 #define HCLK_SPDIF_8CH 314
 #define HCLK_VOP   315
-- 
2.19.1

[PATCH] clk: rockchip: fix I2S1 clock gate register for rk3328

[Katsuhiro Suzuki]

This patch fixes definition of I2S1 clock gate register for rk3328.
Current setting is not related I2S clocks.
  - bit6 of CRU_CLKGATE_CON0 means clk_ddrmon_en
  - bit6 of CRU_CLKGATE_CON1 means clk_i2s1_en

Signed-off-by: Katsuhiro Suzuki 
---
 drivers/clk/rockchip/clk-rk3328.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/rockchip/clk-rk3328.c 
b/drivers/clk/rockchip/clk-rk3328.c
index 2c5426607790..1eb46aa8b2fa 100644
--- a/drivers/clk/rockchip/clk-rk3328.c
+++ b/drivers/clk/rockchip/clk-rk3328.c
@@ -392,7 +392,7 @@ static struct rockchip_clk_branch rk3328_clk_branches[] 
__initdata = {
RK3328_CLKGATE_CON(1), 5, GFLAGS,
_i2s1_fracmux),
GATE(SCLK_I2S1, "clk_i2s1", "i2s1_pre", CLK_SET_RATE_PARENT,
-   RK3328_CLKGATE_CON(0), 6, GFLAGS),
+   RK3328_CLKGATE_CON(1), 6, GFLAGS),
COMPOSITE_NODIV(SCLK_I2S1_OUT, "i2s1_out", mux_i2s1out_p, 0,
RK3328_CLKSEL_CON(8), 12, 1, MFLAGS,
RK3328_CLKGATE_CON(1), 7, GFLAGS),
-- 
2.19.1

[PATCH] clk: rockchip: fix I2S1 clock gate register for rk3328

[Katsuhiro Suzuki]

This patch fixes definition of I2S1 clock gate register for rk3328.
Current setting is not related I2S clocks.
  - bit6 of CRU_CLKGATE_CON0 means clk_ddrmon_en
  - bit6 of CRU_CLKGATE_CON1 means clk_i2s1_en

Signed-off-by: Katsuhiro Suzuki 
---
 drivers/clk/rockchip/clk-rk3328.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/rockchip/clk-rk3328.c 
b/drivers/clk/rockchip/clk-rk3328.c
index 2c5426607790..1eb46aa8b2fa 100644
--- a/drivers/clk/rockchip/clk-rk3328.c
+++ b/drivers/clk/rockchip/clk-rk3328.c
@@ -392,7 +392,7 @@ static struct rockchip_clk_branch rk3328_clk_branches[] 
__initdata = {
RK3328_CLKGATE_CON(1), 5, GFLAGS,
_i2s1_fracmux),
GATE(SCLK_I2S1, "clk_i2s1", "i2s1_pre", CLK_SET_RATE_PARENT,
-   RK3328_CLKGATE_CON(0), 6, GFLAGS),
+   RK3328_CLKGATE_CON(1), 6, GFLAGS),
COMPOSITE_NODIV(SCLK_I2S1_OUT, "i2s1_out", mux_i2s1out_p, 0,
RK3328_CLKSEL_CON(8), 12, 1, MFLAGS,
RK3328_CLKGATE_CON(1), 7, GFLAGS),
-- 
2.19.1

Re: [PATCH] kbuild: rpm-pkg: fix binrpm-pkg breakage when O= is used

[Masahiro Yamada]

Hi Laurent,

On Sun, Nov 18, 2018 at 4:22 AM Laurent Pinchart
 wrote:
>
> Hi Yamada-san,
>
> On Monday, 5 November 2018 09:51:49 EET Masahiro Yamada wrote:
> > Zhenzhong Duan reported that running 'make O=/build/kernel binrpm-pkg'
> > failed with the following errors:
> >
> >   Running 'make O=/build/kernel binrpm-pkg' failed with below two errors.
> >
> >   Makefile:600: include/config/auto.conf: No such file or directory
> >
> >   + cp make -C /mnt/root/kernel O=/build/kernel image_name make -f
> >   /mnt/root/kernel/Makefile ...
> >   cp: invalid option -- 'C'
> >   Try 'cp --help' for more information.
> >
> > Prior to commit 80463f1b7bf9 ("kbuild: add --include-dir flag only
> > for out-of-tree build"), both objtree and srctree were added to
> > --include-dir, and the wrong code 'make image_name' was working
> > relying on that. Now, the potential issue that had previously been
> > hidden just showed up.
> >
> > 'make image_name' recurses to the generated $(objtree)/Makefile and
> > ends up with running in srctree, which is incorrect. It should be
> > invoked with '-f $srctree/Makefile' (or KBUILD_SRC=) to be executed
> > in objtree.
>
> I (painfully) found out that a similar problem occurs with 'make bindeb-pkg'.
>
> After spending half a day unsuccessfully trying to debug and fix the problem I
> ran across this patch. The naive approach of mimicking the solution and adding
> 'MAKE="$MAKE -f $srctree/Makefile"' at the top of scripts/package/builddeb
> didn't work. Reverting commit 80463f1b7bf9 fixed the problem.
>
> Would you be able to give it a look ? I'm afraid my limited skills related to
> the kernel build system don't allow me to submit a fix :-S

I fixed bindeb-pkg as well for v4.20-rc2

commit 02826a6ba301b72461c3706e1cc66d5571cd327e
Author: Masahiro Yamada 
Date:   Mon Nov 5 16:52:34 2018 +0900

kbuild: deb-pkg: fix bindeb-pkg breakage when O= is used


If you still see a problem, please let me know.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH] kbuild: rpm-pkg: fix binrpm-pkg breakage when O= is used

[Masahiro Yamada]

Hi Laurent,

On Sun, Nov 18, 2018 at 4:22 AM Laurent Pinchart
 wrote:
>
> Hi Yamada-san,
>
> On Monday, 5 November 2018 09:51:49 EET Masahiro Yamada wrote:
> > Zhenzhong Duan reported that running 'make O=/build/kernel binrpm-pkg'
> > failed with the following errors:
> >
> >   Running 'make O=/build/kernel binrpm-pkg' failed with below two errors.
> >
> >   Makefile:600: include/config/auto.conf: No such file or directory
> >
> >   + cp make -C /mnt/root/kernel O=/build/kernel image_name make -f
> >   /mnt/root/kernel/Makefile ...
> >   cp: invalid option -- 'C'
> >   Try 'cp --help' for more information.
> >
> > Prior to commit 80463f1b7bf9 ("kbuild: add --include-dir flag only
> > for out-of-tree build"), both objtree and srctree were added to
> > --include-dir, and the wrong code 'make image_name' was working
> > relying on that. Now, the potential issue that had previously been
> > hidden just showed up.
> >
> > 'make image_name' recurses to the generated $(objtree)/Makefile and
> > ends up with running in srctree, which is incorrect. It should be
> > invoked with '-f $srctree/Makefile' (or KBUILD_SRC=) to be executed
> > in objtree.
>
> I (painfully) found out that a similar problem occurs with 'make bindeb-pkg'.
>
> After spending half a day unsuccessfully trying to debug and fix the problem I
> ran across this patch. The naive approach of mimicking the solution and adding
> 'MAKE="$MAKE -f $srctree/Makefile"' at the top of scripts/package/builddeb
> didn't work. Reverting commit 80463f1b7bf9 fixed the problem.
>
> Would you be able to give it a look ? I'm afraid my limited skills related to
> the kernel build system don't allow me to submit a fix :-S

I fixed bindeb-pkg as well for v4.20-rc2

commit 02826a6ba301b72461c3706e1cc66d5571cd327e
Author: Masahiro Yamada 
Date:   Mon Nov 5 16:52:34 2018 +0900

kbuild: deb-pkg: fix bindeb-pkg breakage when O= is used


If you still see a problem, please let me know.



-- 
Best Regards
Masahiro Yamada

[no subject]

[Major Dennis Hornbeck]

I am in the military unit here in Afghanistan, we have some amount of funds 
that we want to move out of the country. My partners and I need a good partner 
someone we can trust. It is risk free and legal. Reply to this email: 
majordennis...@gmail.com



Regards,Major Dennis Hornbeck.

[no subject]

[Major Dennis Hornbeck]

I am in the military unit here in Afghanistan, we have some amount of funds 
that we want to move out of the country. My partners and I need a good partner 
someone we can trust. It is risk free and legal. Reply to this email: 
majordennis...@gmail.com



Regards,Major Dennis Hornbeck.

[no subject]

[Major Dennis Hornbeck]

I am in the military unit here in Afghanistan, we have some amount of funds 
that we want to move out of the country. My partners and I need a good partner 
someone we can trust. It is risk free and legal. Reply to this email: 
majordennis...@gmail.com



Regards,Major Dennis Hornbeck.

[no subject]

[Major Dennis Hornbeck]

I am in the military unit here in Afghanistan, we have some amount of funds 
that we want to move out of the country. My partners and I need a good partner 
someone we can trust. It is risk free and legal. Reply to this email: 
majordennis...@gmail.com



Regards,Major Dennis Hornbeck.

Re: [PATCH V2 1/2] clocksource: imx-gpt: add support for ARM64

[Daniel Lezcano]

On 05/11/2018 02:10, Anson Huang wrote:
> This patch allows building and compile-testing the i.MX
> GPT driver also for ARM64. The delay_timer is only
> supported on ARMv7.
> 
> Signed-off-by: Anson Huang 
> ---
> no change since V1.

Applied, thanks.

  -- Daniel


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH V2 1/2] clocksource: imx-gpt: add support for ARM64

[Daniel Lezcano]

On 05/11/2018 02:10, Anson Huang wrote:
> This patch allows building and compile-testing the i.MX
> GPT driver also for ARM64. The delay_timer is only
> supported on ARMv7.
> 
> Signed-off-by: Anson Huang 
> ---
> no change since V1.

Applied, thanks.

  -- Daniel


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak

[Daniel Lezcano]

Hi Anson,

On 06/11/2018 10:12, Anson Huang wrote:

[ ... ]

 Please convert to timer-of API.
>>>
>>> This patch mainly to fix the potential resource leak issue, for
>>> converting to timer-of API, should I add another patch for it? Since
>>> it is a different subject.
>>
>> Actually I was unclear. The patch is duplicating the kfree in the error path,
>> usually a rollback routine should be provided.
>>
>> So instead of creating this rollback path, you can directly use the timer-of
>> which contains the error check and rollback.
> 
> I understand the timer-of can save some code in timer driver, but current GPT
> driver is kind of complicated in order to support so many i.MX platforms with
> different GPT design, change it to timer-of will need some effort and I think 
> we
> can do it later. If the duplicating kfree in error path confuses you, I can 
> change
> it to rollback path you said. Below are 2 options in my mind:
> 
> 1. add kfree to rollback path;
> 2. skip this patch, just apply 1/2 patch, and let me find some time to change 
> GPT
> driver using timer-of. 
> 
> Which one do you prefer? Thanks.

I'm fine with option 2.

Thanks

  -- Daniel

-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH V2 2/2] clocksource: imx-gpt: add necessary kfree to avoid resource leak

[Daniel Lezcano]

Hi Anson,

On 06/11/2018 10:12, Anson Huang wrote:

[ ... ]

 Please convert to timer-of API.
>>>
>>> This patch mainly to fix the potential resource leak issue, for
>>> converting to timer-of API, should I add another patch for it? Since
>>> it is a different subject.
>>
>> Actually I was unclear. The patch is duplicating the kfree in the error path,
>> usually a rollback routine should be provided.
>>
>> So instead of creating this rollback path, you can directly use the timer-of
>> which contains the error check and rollback.
> 
> I understand the timer-of can save some code in timer driver, but current GPT
> driver is kind of complicated in order to support so many i.MX platforms with
> different GPT design, change it to timer-of will need some effort and I think 
> we
> can do it later. If the duplicating kfree in error path confuses you, I can 
> change
> it to rollback path you said. Below are 2 options in my mind:
> 
> 1. add kfree to rollback path;
> 2. skip this patch, just apply 1/2 patch, and let me find some time to change 
> GPT
> driver using timer-of. 
> 
> Which one do you prefer? Thanks.

I'm fine with option 2.

Thanks

  -- Daniel

-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH v2] riscv: add asm/unistd.h UAPI header

[Nick Kossifidis]

Hello David,

Στις 2018-11-08 21:02, David Abdurachmanov έγραψε:
Marcin Juszkiewicz reported issues while generating syscall table for 
riscv
using 4.20-rc1. The patch refactors our unistd.h files to match some 
other

architectures.

- Add asm/unistd.h UAPI header, which has __ARCH_WANT_NEW_STAT only for 
64-bit

- Remove asm/syscalls.h UAPI header and merge to asm/unistd.h
- Adjust kernel asm/unistd.h

So now asm/unistd.h UAPI header should show all syscalls for riscv.

Before this, Makefile simply put `#include ` into
generated asm/unistd.h UAPI header thus user didn't see:

- __NR_riscv_flush_icache
- __NR_newfstatat
- __NR_fstat

which are supported by riscv kernel.

Signed-off-by: David Abdurachmanov 
Cc: Arnd Bergmann 
Cc: Marcin Juszkiewicz 
Cc: Guenter Roeck 
Fixes: 67314ec7b025 ("RISC-V: Request newstat syscalls")
Signed-off-by: David Abdurachmanov 
---
 arch/riscv/include/asm/unistd.h|  5 ++--
 arch/riscv/include/uapi/asm/syscalls.h | 29 --
 arch/riscv/include/uapi/asm/unistd.h   | 41 ++
 3 files changed, 43 insertions(+), 32 deletions(-)
 delete mode 100644 arch/riscv/include/uapi/asm/syscalls.h
 create mode 100644 arch/riscv/include/uapi/asm/unistd.h

diff --git a/arch/riscv/include/asm/unistd.h 
b/arch/riscv/include/asm/unistd.h

index eff7aa9aa163..fef96f117b4d 100644
--- a/arch/riscv/include/asm/unistd.h
+++ b/arch/riscv/include/asm/unistd.h
@@ -13,10 +13,9 @@

 /*
  * There is explicitly no include guard here because this file is 
expected to

- * be included multiple times.  See uapi/asm/syscalls.h for more info.
+ * be included multiple times.
  */

-#define __ARCH_WANT_NEW_STAT
 #define __ARCH_WANT_SYS_CLONE
+
 #include 
-#include 
diff --git a/arch/riscv/include/uapi/asm/syscalls.h
b/arch/riscv/include/uapi/asm/syscalls.h
deleted file mode 100644
index 206dc4b0f6ea..
--- a/arch/riscv/include/uapi/asm/syscalls.h
+++ /dev/null
@@ -1,29 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Copyright (C) 2017-2018 SiFive
- */
-
-/*
- * There is explicitly no include guard here because this file is 
expected to
- * be included multiple times in order to define the syscall macros 
via

- * __SYSCALL.
- */
-
-/*
- * Allows the instruction cache to be flushed from userspace.  Despite 
RISC-V
- * having a direct 'fence.i' instruction available to userspace (which 
we
- * can't trap!), that's not actually viable when running on Linux 
because the
- * kernel might schedule a process on another hart.  There is no way 
for
- * userspace to handle this without invoking the kernel (as it doesn't 
know the
- * thread->hart mappings), so we've defined a RISC-V specific system 
call to

- * flush the instruction cache.
- *
- * __NR_riscv_flush_icache is defined to flush the instruction cache 
over an
- * address range, with the flush applying to either all threads or 
just the
- * caller.  We don't currently do anything with the address range, 
that's just

- * in there for forwards compatibility.
- */
-#ifndef __NR_riscv_flush_icache
-#define __NR_riscv_flush_icache (__NR_arch_specific_syscall + 15)
-#endif
-__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache)
diff --git a/arch/riscv/include/uapi/asm/unistd.h
b/arch/riscv/include/uapi/asm/unistd.h
new file mode 100644
index ..1f3bd3ebbb0d
--- /dev/null
+++ b/arch/riscv/include/uapi/asm/unistd.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Copyright (C) 2018 David Abdurachmanov 


+ *
+ * This program is free software; you can redistribute it and/or 
modify

+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see 
.

+ */
+
+#ifdef __LP64__
+#define __ARCH_WANT_NEW_STAT
+#endif /* __LP64__ */
+
+#include 
+
+/*
+ * Allows the instruction cache to be flushed from userspace.  Despite 
RISC-V
+ * having a direct 'fence.i' instruction available to userspace (which 
we
+ * can't trap!), that's not actually viable when running on Linux 
because the
+ * kernel might schedule a process on another hart.  There is no way 
for
+ * userspace to handle this without invoking the kernel (as it doesn't 
know the
+ * thread->hart mappings), so we've defined a RISC-V specific system 
call to

+ * flush the instruction cache.
+ *
+ * __NR_riscv_flush_icache is defined to flush the instruction cache 
over an
+ * address range, with the flush applying to either all threads or 
just the
+ * caller.  We don't currently do anything with the address range, 
that's just

+ * in there for forwards 

Re: [PATCH v2] riscv: add asm/unistd.h UAPI header

[Nick Kossifidis]

Hello David,

Στις 2018-11-08 21:02, David Abdurachmanov έγραψε:
Marcin Juszkiewicz reported issues while generating syscall table for 
riscv
using 4.20-rc1. The patch refactors our unistd.h files to match some 
other

architectures.

- Add asm/unistd.h UAPI header, which has __ARCH_WANT_NEW_STAT only for 
64-bit

- Remove asm/syscalls.h UAPI header and merge to asm/unistd.h
- Adjust kernel asm/unistd.h

So now asm/unistd.h UAPI header should show all syscalls for riscv.

Before this, Makefile simply put `#include ` into
generated asm/unistd.h UAPI header thus user didn't see:

- __NR_riscv_flush_icache
- __NR_newfstatat
- __NR_fstat

which are supported by riscv kernel.

Signed-off-by: David Abdurachmanov 
Cc: Arnd Bergmann 
Cc: Marcin Juszkiewicz 
Cc: Guenter Roeck 
Fixes: 67314ec7b025 ("RISC-V: Request newstat syscalls")
Signed-off-by: David Abdurachmanov 
---
 arch/riscv/include/asm/unistd.h|  5 ++--
 arch/riscv/include/uapi/asm/syscalls.h | 29 --
 arch/riscv/include/uapi/asm/unistd.h   | 41 ++
 3 files changed, 43 insertions(+), 32 deletions(-)
 delete mode 100644 arch/riscv/include/uapi/asm/syscalls.h
 create mode 100644 arch/riscv/include/uapi/asm/unistd.h

diff --git a/arch/riscv/include/asm/unistd.h 
b/arch/riscv/include/asm/unistd.h

index eff7aa9aa163..fef96f117b4d 100644
--- a/arch/riscv/include/asm/unistd.h
+++ b/arch/riscv/include/asm/unistd.h
@@ -13,10 +13,9 @@

 /*
  * There is explicitly no include guard here because this file is 
expected to

- * be included multiple times.  See uapi/asm/syscalls.h for more info.
+ * be included multiple times.
  */

-#define __ARCH_WANT_NEW_STAT
 #define __ARCH_WANT_SYS_CLONE
+
 #include 
-#include 
diff --git a/arch/riscv/include/uapi/asm/syscalls.h
b/arch/riscv/include/uapi/asm/syscalls.h
deleted file mode 100644
index 206dc4b0f6ea..
--- a/arch/riscv/include/uapi/asm/syscalls.h
+++ /dev/null
@@ -1,29 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Copyright (C) 2017-2018 SiFive
- */
-
-/*
- * There is explicitly no include guard here because this file is 
expected to
- * be included multiple times in order to define the syscall macros 
via

- * __SYSCALL.
- */
-
-/*
- * Allows the instruction cache to be flushed from userspace.  Despite 
RISC-V
- * having a direct 'fence.i' instruction available to userspace (which 
we
- * can't trap!), that's not actually viable when running on Linux 
because the
- * kernel might schedule a process on another hart.  There is no way 
for
- * userspace to handle this without invoking the kernel (as it doesn't 
know the
- * thread->hart mappings), so we've defined a RISC-V specific system 
call to

- * flush the instruction cache.
- *
- * __NR_riscv_flush_icache is defined to flush the instruction cache 
over an
- * address range, with the flush applying to either all threads or 
just the
- * caller.  We don't currently do anything with the address range, 
that's just

- * in there for forwards compatibility.
- */
-#ifndef __NR_riscv_flush_icache
-#define __NR_riscv_flush_icache (__NR_arch_specific_syscall + 15)
-#endif
-__SYSCALL(__NR_riscv_flush_icache, sys_riscv_flush_icache)
diff --git a/arch/riscv/include/uapi/asm/unistd.h
b/arch/riscv/include/uapi/asm/unistd.h
new file mode 100644
index ..1f3bd3ebbb0d
--- /dev/null
+++ b/arch/riscv/include/uapi/asm/unistd.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Copyright (C) 2018 David Abdurachmanov 


+ *
+ * This program is free software; you can redistribute it and/or 
modify

+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see 
.

+ */
+
+#ifdef __LP64__
+#define __ARCH_WANT_NEW_STAT
+#endif /* __LP64__ */
+
+#include 
+
+/*
+ * Allows the instruction cache to be flushed from userspace.  Despite 
RISC-V
+ * having a direct 'fence.i' instruction available to userspace (which 
we
+ * can't trap!), that's not actually viable when running on Linux 
because the
+ * kernel might schedule a process on another hart.  There is no way 
for
+ * userspace to handle this without invoking the kernel (as it doesn't 
know the
+ * thread->hart mappings), so we've defined a RISC-V specific system 
call to

+ * flush the instruction cache.
+ *
+ * __NR_riscv_flush_icache is defined to flush the instruction cache 
over an
+ * address range, with the flush applying to either all threads or 
just the
+ * caller.  We don't currently do anything with the address range, 
that's just

+ * in there for forwards 

Re: [RFC v2 5/7] clocksource/drivers/rtl8186: Add RTL8186 timer driver

[Daniel Lezcano]

Hi Yasha,

except the few details below, the driver looks good to me.

On 01/10/2018 12:29, Yasha Cherikovsky wrote:
> The Realtek RTL8186 SoC is a MIPS based SoC
> used in some home routers [1][2].
> 
> This adds a driver to handle the built-in timers
> on this SoC.
> 
> Timers 0 and 1 are 24bit timers.
> Timers 2 and 3 are 32bit timers.
> 
> Use Timer2 as clocksource and Timer3 for clockevents.
> Timer2 is also used for sched_clock.
> 
> [1] https://www.linux-mips.org/wiki/Realtek_SOC#Realtek_RTL8186
> [2] https://wikidevi.com/wiki/Realtek_RTL8186
> 
> Signed-off-by: Yasha Cherikovsky 
> Cc: Ralf Baechle 
> Cc: Paul Burton 
> Cc: James Hogan 
> Cc: Daniel Lezcano 
> Cc: Thomas Gleixner 
> Cc: Rob Herring 
> Cc: Mark Rutland 
> Cc: linux-m...@linux-mips.org
> Cc: devicet...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/clocksource/Kconfig |   9 ++
>  drivers/clocksource/Makefile|   1 +
>  drivers/clocksource/timer-rtl8186.c | 220 
>  3 files changed, 230 insertions(+)
>  create mode 100644 drivers/clocksource/timer-rtl8186.c
> 
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index dec0dd88ec15..da87f73d0631 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -609,4 +609,13 @@ config ATCPIT100_TIMER
>   help
> This option enables support for the Andestech ATCPIT100 timers.
>  
> +config RTL8186_TIMER
> + bool "RTL8186 timer driver"
> + depends on MACH_RTL8186
> + depends on COMMON_CLK
> + select TIMER_OF
> + select CLKSRC_MMIO
> + help
> +   Enables support for the RTL8186 timer driver.
> +

Please, convert this entry like the MTK_TIMER or the SPRD_TIMER.

>  endmenu
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index 00caf37e52f9..734e8566e1b6 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -78,3 +78,4 @@ obj-$(CONFIG_H8300_TPU) += h8300_tpu.o
>  obj-$(CONFIG_CLKSRC_ST_LPC)  += clksrc_st_lpc.o
>  obj-$(CONFIG_X86_NUMACHIP)   += numachip.o
>  obj-$(CONFIG_ATCPIT100_TIMER)+= timer-atcpit100.o
> +obj-$(CONFIG_RTL8186_TIMER)  += timer-rtl8186.o
> diff --git a/drivers/clocksource/timer-rtl8186.c 
> b/drivers/clocksource/timer-rtl8186.c
> new file mode 100644
> index ..47ef4b09ad27
> --- /dev/null
> +++ b/drivers/clocksource/timer-rtl8186.c
> @@ -0,0 +1,220 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Realtek RTL8186 SoC timer driver.
> + *
> + * Timer0 (24bit): Unused
> + * Timer1 (24bit): Unused
> + * Timer2 (32bit): Used as clocksource
> + * Timer3 (32bit): Used as clock event device
> + *
> + * Copyright (C) 2018 Yasha Cherikovsky
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 

Why do you need those 2 includes above ?

> +#include "timer-of.h"
> +
> +/* Timer registers */
> +#define TCCNR0x0
> +#define TCIR 0x4
> +#define TC_DATA(t)   (0x10 + 4 * (t))
> +#define TC_CNT(t)(0x20 + 4 * (t))
> +
> +/* TCCNR register bits */
> +#define TCCNR_TC_EN_BIT(t)   BIT((t) * 2)
> +#define TCCNR_TC_MODE_BIT(t) BIT((t) * 2 + 1)
> +#define TCCNR_TC_SRC_BIT(t)  BIT((t) + 8)
> +
> +/* TCIR register bits */
> +#define TCIR_TC_IE_BIT(t)BIT(t)
> +#define TCIR_TC_IP_BIT(t)BIT((t) + 4)
> +
> +
> +/* Forward declaration */
> +static struct timer_of to;
> +
> +static void __iomem *base;
> +
> +

nit: extra line

> +#define RTL8186_TIMER_MODE_COUNTER   0
> +#define RTL8186_TIMER_MODE_TIMER 1
> +

[ ... ]

Thanks

  -- Daniel



-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [RFC v2 5/7] clocksource/drivers/rtl8186: Add RTL8186 timer driver

[Daniel Lezcano]

Hi Yasha,

except the few details below, the driver looks good to me.

On 01/10/2018 12:29, Yasha Cherikovsky wrote:
> The Realtek RTL8186 SoC is a MIPS based SoC
> used in some home routers [1][2].
> 
> This adds a driver to handle the built-in timers
> on this SoC.
> 
> Timers 0 and 1 are 24bit timers.
> Timers 2 and 3 are 32bit timers.
> 
> Use Timer2 as clocksource and Timer3 for clockevents.
> Timer2 is also used for sched_clock.
> 
> [1] https://www.linux-mips.org/wiki/Realtek_SOC#Realtek_RTL8186
> [2] https://wikidevi.com/wiki/Realtek_RTL8186
> 
> Signed-off-by: Yasha Cherikovsky 
> Cc: Ralf Baechle 
> Cc: Paul Burton 
> Cc: James Hogan 
> Cc: Daniel Lezcano 
> Cc: Thomas Gleixner 
> Cc: Rob Herring 
> Cc: Mark Rutland 
> Cc: linux-m...@linux-mips.org
> Cc: devicet...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/clocksource/Kconfig |   9 ++
>  drivers/clocksource/Makefile|   1 +
>  drivers/clocksource/timer-rtl8186.c | 220 
>  3 files changed, 230 insertions(+)
>  create mode 100644 drivers/clocksource/timer-rtl8186.c
> 
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index dec0dd88ec15..da87f73d0631 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -609,4 +609,13 @@ config ATCPIT100_TIMER
>   help
> This option enables support for the Andestech ATCPIT100 timers.
>  
> +config RTL8186_TIMER
> + bool "RTL8186 timer driver"
> + depends on MACH_RTL8186
> + depends on COMMON_CLK
> + select TIMER_OF
> + select CLKSRC_MMIO
> + help
> +   Enables support for the RTL8186 timer driver.
> +

Please, convert this entry like the MTK_TIMER or the SPRD_TIMER.

>  endmenu
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index 00caf37e52f9..734e8566e1b6 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -78,3 +78,4 @@ obj-$(CONFIG_H8300_TPU) += h8300_tpu.o
>  obj-$(CONFIG_CLKSRC_ST_LPC)  += clksrc_st_lpc.o
>  obj-$(CONFIG_X86_NUMACHIP)   += numachip.o
>  obj-$(CONFIG_ATCPIT100_TIMER)+= timer-atcpit100.o
> +obj-$(CONFIG_RTL8186_TIMER)  += timer-rtl8186.o
> diff --git a/drivers/clocksource/timer-rtl8186.c 
> b/drivers/clocksource/timer-rtl8186.c
> new file mode 100644
> index ..47ef4b09ad27
> --- /dev/null
> +++ b/drivers/clocksource/timer-rtl8186.c
> @@ -0,0 +1,220 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Realtek RTL8186 SoC timer driver.
> + *
> + * Timer0 (24bit): Unused
> + * Timer1 (24bit): Unused
> + * Timer2 (32bit): Used as clocksource
> + * Timer3 (32bit): Used as clock event device
> + *
> + * Copyright (C) 2018 Yasha Cherikovsky
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 

Why do you need those 2 includes above ?

> +#include "timer-of.h"
> +
> +/* Timer registers */
> +#define TCCNR0x0
> +#define TCIR 0x4
> +#define TC_DATA(t)   (0x10 + 4 * (t))
> +#define TC_CNT(t)(0x20 + 4 * (t))
> +
> +/* TCCNR register bits */
> +#define TCCNR_TC_EN_BIT(t)   BIT((t) * 2)
> +#define TCCNR_TC_MODE_BIT(t) BIT((t) * 2 + 1)
> +#define TCCNR_TC_SRC_BIT(t)  BIT((t) + 8)
> +
> +/* TCIR register bits */
> +#define TCIR_TC_IE_BIT(t)BIT(t)
> +#define TCIR_TC_IP_BIT(t)BIT((t) + 4)
> +
> +
> +/* Forward declaration */
> +static struct timer_of to;
> +
> +static void __iomem *base;
> +
> +

nit: extra line

> +#define RTL8186_TIMER_MODE_COUNTER   0
> +#define RTL8186_TIMER_MODE_TIMER 1
> +

[ ... ]

Thanks

  -- Daniel



-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH 1/2] clocksource: Demote dbx500 PRCMU clocksource

[Daniel Lezcano]

On 15/11/2018 14:32, Linus Walleij wrote:
> Demote the DBx500 PRCMU clocksource to quality 100 and
> mark it as NONSTOP so it will still be used for
> timekeeping across suspend/resume.
> 
> The Nomadik MTU timer which has higher precision will
> be used when the system is up and running, thanks to
> the recent changes properly utilizing the suspend
> clocksources.
> 
> This was discussed back in 2011 when the driver was
> written, but the infrastructure was not available
> upstream to use this timer properly. Now the
> infrastructure is there, so let's finalize the work.
> 
> Cc: Baolin Wang 
> Signed-off-by: Linus Walleij 
> ---

Both applied, thanks!


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH 1/2] clocksource: Demote dbx500 PRCMU clocksource

[Daniel Lezcano]

On 15/11/2018 14:32, Linus Walleij wrote:
> Demote the DBx500 PRCMU clocksource to quality 100 and
> mark it as NONSTOP so it will still be used for
> timekeeping across suspend/resume.
> 
> The Nomadik MTU timer which has higher precision will
> be used when the system is up and running, thanks to
> the recent changes properly utilizing the suspend
> clocksources.
> 
> This was discussed back in 2011 when the driver was
> written, but the infrastructure was not available
> upstream to use this timer properly. Now the
> infrastructure is there, so let's finalize the work.
> 
> Cc: Baolin Wang 
> Signed-off-by: Linus Walleij 
> ---

Both applied, thanks!


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH v2 0/2] clocksource/meson6_timer: implement ARM delay timer

[Daniel Lezcano]

On 15/11/2018 23:46, Martin Blumenstingl wrote:
> While trying to add support for the ARM TWD Timer and the ARM Global
> Timer on Meson8, Meson8b and Meson8m2 (ARM Cortex-A5 and Cortex-A9 SoCs)
> I did a review of the existing driver.
> Unfortunately I found it hard to review because the pre-processor
> #defines did not match the names from the public S805 datasheet. Thus
> patch #1 adjusts these. No functional changes here, this is just
> preparation work for patch #2.
> 
> Using the ARM Global Timer (drivers/clocksource/arm_global_timer.c)
> would have given us a timer-based delay implementation (so udelay() and
> friends would use the timer instead of using a loop-based delay
> implementation). Unfortunately we can't use the ARM Global Timer yet
> because it's input clock is derived from the CPU clock (which can change
> once we enable CPU frequency scaling on these SoCs, for which I will be
> sending patches in the near future).
> Amlogic's 3.10 kernel uses Timer E as delay timer which (with the
> current configuration) has a resolution of 1us. So patch #2 uses
> register_current_timer_delay() to register Timer E as ARM delay timer
> (which will be especially useful as we have to use udelay() when
> changing the CPU clocks during DVFS).
> 
> 
> Changes since v1 at [0]:
> - convert the enums for the input clock (meson6_timera_input_clock and
>   meson6_timere_input_clock) to simple #defines as these are register
>   values and not something driver-internal. All other register values
>   are #defines so it makes sense that these are #defines as well.
> 
> 
> [0] https://patchwork.kernel.org/cover/10658591/
> 
> 
> Martin Blumenstingl (2):
>   clocksource: meson6_timer: use register names from the datasheet
>   clocksource: meson6_timer: implement ARM delay timer
> 
>  drivers/clocksource/meson6_timer.c | 128 +++--
>  1 file changed, 85 insertions(+), 43 deletions(-)

Both applied, thanks!


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH v2 0/2] clocksource/meson6_timer: implement ARM delay timer

[Daniel Lezcano]

On 15/11/2018 23:46, Martin Blumenstingl wrote:
> While trying to add support for the ARM TWD Timer and the ARM Global
> Timer on Meson8, Meson8b and Meson8m2 (ARM Cortex-A5 and Cortex-A9 SoCs)
> I did a review of the existing driver.
> Unfortunately I found it hard to review because the pre-processor
> #defines did not match the names from the public S805 datasheet. Thus
> patch #1 adjusts these. No functional changes here, this is just
> preparation work for patch #2.
> 
> Using the ARM Global Timer (drivers/clocksource/arm_global_timer.c)
> would have given us a timer-based delay implementation (so udelay() and
> friends would use the timer instead of using a loop-based delay
> implementation). Unfortunately we can't use the ARM Global Timer yet
> because it's input clock is derived from the CPU clock (which can change
> once we enable CPU frequency scaling on these SoCs, for which I will be
> sending patches in the near future).
> Amlogic's 3.10 kernel uses Timer E as delay timer which (with the
> current configuration) has a resolution of 1us. So patch #2 uses
> register_current_timer_delay() to register Timer E as ARM delay timer
> (which will be especially useful as we have to use udelay() when
> changing the CPU clocks during DVFS).
> 
> 
> Changes since v1 at [0]:
> - convert the enums for the input clock (meson6_timera_input_clock and
>   meson6_timere_input_clock) to simple #defines as these are register
>   values and not something driver-internal. All other register values
>   are #defines so it makes sense that these are #defines as well.
> 
> 
> [0] https://patchwork.kernel.org/cover/10658591/
> 
> 
> Martin Blumenstingl (2):
>   clocksource: meson6_timer: use register names from the datasheet
>   clocksource: meson6_timer: implement ARM delay timer
> 
>  drivers/clocksource/meson6_timer.c | 128 +++--
>  1 file changed, 85 insertions(+), 43 deletions(-)

Both applied, thanks!


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH] clockevents/drivers/tegra20: Remove obsolete inclusion of

[Daniel Lezcano]

On 15/11/2018 11:00, Geert Uytterhoeven wrote:
> As of commit da4a686a2cfb077a ("ARM: smp_twd: convert to use CLKSRC_OF
> init"), this header file is no longer used.
> 
> Signed-off-by: Geert Uytterhoeven 
> ---
Applied.

Thanks!

-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH] clockevents/drivers/tegra20: Remove obsolete inclusion of

[Daniel Lezcano]

On 15/11/2018 11:00, Geert Uytterhoeven wrote:
> As of commit da4a686a2cfb077a ("ARM: smp_twd: convert to use CLKSRC_OF
> init"), this header file is no longer used.
> 
> Signed-off-by: Geert Uytterhoeven 
> ---
Applied.

Thanks!

-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH] mm: use this_cpu_cmpxchg_double in put_cpu_partial

[Wei Yang]

On Fri, Nov 16, 2018 at 05:33:35PM -0800, Wengang Wang wrote:
>The this_cpu_cmpxchg makes the do-while loop pass as long as the
>s->cpu_slab->partial as the same value. It doesn't care what happened to
>that slab. Interrupt is not disabled, and new alloc/free can happen in the

Well, I seems to understand your description.

There are two slabs

   * one which put_cpu_partial() trying to free an object
   * one which is the first slab in cpu_partial list

There is some tricky case, the first slab in cpu_partial list we
reference to will change since interrupt is not disabled.

>interrupt handlers. Theoretically, after we have a reference to the it,

 ^^^
 one more word?

>stored in _oldpage_, the first slab on the partial list on this CPU can be

^^^
One little suggestion here, mayby use cpu_partial would be more easy to
understand. I confused this with the partial list in kmem_cache_node at
the first time.  :-)

>moved to kmem_cache_node and then moved to different kmem_cache_cpu and
>then somehow can be added back as head to partial list of current
>kmem_cache_cpu, though that is a very rare case. If that rare case really

Actually, no matter what happens after the removal of the first slab in
cpu_partial, it would leads to problem.

>happened, the reading of oldpage->pobjects may get a 0xdead
>unexpectedly, stored in _pobjects_, if the reading happens just after
>another CPU removed the slab from kmem_cache_node, setting lru.prev to
>LIST_POISON2 (0xdead0200). The wrong _pobjects_(negative) then
>prevents slabs from being moved to kmem_cache_node and being finally freed.
>
>We see in a vmcore, there are 375210 slabs kept in the partial list of one
>kmem_cache_cpu, but only 305 in-use objects in the same list for
>kmalloc-2048 cache. We see negative values for page.pobjects, the last page
>with negative _pobjects_ has the value of 0xdead0004, the next page looks
>good (_pobjects is 1).
>
>For the fix, I wanted to call this_cpu_cmpxchg_double with
>oldpage->pobjects, but failed due to size difference between
>oldpage->pobjects and cpu_slab->partial. So I changed to call
>this_cpu_cmpxchg_double with _tid_. I don't really want no alloc/free
>happen in between, but just want to make sure the first slab did expereince
>a remove and re-add. This patch is more to call for ideas.

Maybe not an exact solution.

I took a look into the code and change log.

_tid_ is introduced by commit 8a5ec0ba42c4 ('Lockless (and preemptless)
fastpaths for slub'), which is used to guard cpu_freelist. While we don't
modify _tid_ when cpu_partial changes.

May need another _tid_ for cpu_partial?

>
>Signed-off-by: Wengang Wang 
>---
> mm/slub.c | 20 +---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
>diff --git a/mm/slub.c b/mm/slub.c
>index e3629cd..26539e6 100644
>--- a/mm/slub.c
>+++ b/mm/slub.c
>@@ -2248,6 +2248,7 @@ static void put_cpu_partial(struct kmem_cache *s, struct 
>page *page, int drain)
> {
> #ifdef CONFIG_SLUB_CPU_PARTIAL
>   struct page *oldpage;
>+  unsigned long tid;
>   int pages;
>   int pobjects;
> 
>@@ -2255,8 +2256,12 @@ static void put_cpu_partial(struct kmem_cache *s, 
>struct page *page, int drain)
>   do {
>   pages = 0;
>   pobjects = 0;
>-  oldpage = this_cpu_read(s->cpu_slab->partial);
> 
>+  tid = this_cpu_read(s->cpu_slab->tid);
>+  /* read tid before reading oldpage */
>+  barrier();
>+
>+  oldpage = this_cpu_read(s->cpu_slab->partial);
>   if (oldpage) {
>   pobjects = oldpage->pobjects;
>   pages = oldpage->pages;
>@@ -2283,8 +2288,17 @@ static void put_cpu_partial(struct kmem_cache *s, 
>struct page *page, int drain)
>   page->pobjects = pobjects;
>   page->next = oldpage;
> 
>-  } while (this_cpu_cmpxchg(s->cpu_slab->partial, oldpage, page)
>-  != oldpage);
>+  /* we dont' change tid, but want to make sure it didn't change
>+   * in between. We don't really hope alloc/free not happen on
>+   * this CPU, but don't want the first slab be removed from and
>+   * then re-added as head to this partial list. If that case
>+   * happened, pobjects may read 0xdead when this slab is just
>+   * removed from kmem_cache_node by other CPU setting lru.prev
>+   * to LIST_POISON2.
>+   */
>+  } while (this_cpu_cmpxchg_double(s->cpu_slab->partial, s->cpu_slab->tid,
>+   oldpage, tid, page, tid) == 0);
>+
>   if (unlikely(!s->cpu_partial)) {
>   unsigned long flags;
> 
>-- 
>2.9.5

-- 
Wei Yang
Help you, Help me

Re: [PATCH] mm: use this_cpu_cmpxchg_double in put_cpu_partial

[Wei Yang]

On Fri, Nov 16, 2018 at 05:33:35PM -0800, Wengang Wang wrote:
>The this_cpu_cmpxchg makes the do-while loop pass as long as the
>s->cpu_slab->partial as the same value. It doesn't care what happened to
>that slab. Interrupt is not disabled, and new alloc/free can happen in the

Well, I seems to understand your description.

There are two slabs

   * one which put_cpu_partial() trying to free an object
   * one which is the first slab in cpu_partial list

There is some tricky case, the first slab in cpu_partial list we
reference to will change since interrupt is not disabled.

>interrupt handlers. Theoretically, after we have a reference to the it,

 ^^^
 one more word?

>stored in _oldpage_, the first slab on the partial list on this CPU can be

^^^
One little suggestion here, mayby use cpu_partial would be more easy to
understand. I confused this with the partial list in kmem_cache_node at
the first time.  :-)

>moved to kmem_cache_node and then moved to different kmem_cache_cpu and
>then somehow can be added back as head to partial list of current
>kmem_cache_cpu, though that is a very rare case. If that rare case really

Actually, no matter what happens after the removal of the first slab in
cpu_partial, it would leads to problem.

>happened, the reading of oldpage->pobjects may get a 0xdead
>unexpectedly, stored in _pobjects_, if the reading happens just after
>another CPU removed the slab from kmem_cache_node, setting lru.prev to
>LIST_POISON2 (0xdead0200). The wrong _pobjects_(negative) then
>prevents slabs from being moved to kmem_cache_node and being finally freed.
>
>We see in a vmcore, there are 375210 slabs kept in the partial list of one
>kmem_cache_cpu, but only 305 in-use objects in the same list for
>kmalloc-2048 cache. We see negative values for page.pobjects, the last page
>with negative _pobjects_ has the value of 0xdead0004, the next page looks
>good (_pobjects is 1).
>
>For the fix, I wanted to call this_cpu_cmpxchg_double with
>oldpage->pobjects, but failed due to size difference between
>oldpage->pobjects and cpu_slab->partial. So I changed to call
>this_cpu_cmpxchg_double with _tid_. I don't really want no alloc/free
>happen in between, but just want to make sure the first slab did expereince
>a remove and re-add. This patch is more to call for ideas.

Maybe not an exact solution.

I took a look into the code and change log.

_tid_ is introduced by commit 8a5ec0ba42c4 ('Lockless (and preemptless)
fastpaths for slub'), which is used to guard cpu_freelist. While we don't
modify _tid_ when cpu_partial changes.

May need another _tid_ for cpu_partial?

>
>Signed-off-by: Wengang Wang 
>---
> mm/slub.c | 20 +---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
>diff --git a/mm/slub.c b/mm/slub.c
>index e3629cd..26539e6 100644
>--- a/mm/slub.c
>+++ b/mm/slub.c
>@@ -2248,6 +2248,7 @@ static void put_cpu_partial(struct kmem_cache *s, struct 
>page *page, int drain)
> {
> #ifdef CONFIG_SLUB_CPU_PARTIAL
>   struct page *oldpage;
>+  unsigned long tid;
>   int pages;
>   int pobjects;
> 
>@@ -2255,8 +2256,12 @@ static void put_cpu_partial(struct kmem_cache *s, 
>struct page *page, int drain)
>   do {
>   pages = 0;
>   pobjects = 0;
>-  oldpage = this_cpu_read(s->cpu_slab->partial);
> 
>+  tid = this_cpu_read(s->cpu_slab->tid);
>+  /* read tid before reading oldpage */
>+  barrier();
>+
>+  oldpage = this_cpu_read(s->cpu_slab->partial);
>   if (oldpage) {
>   pobjects = oldpage->pobjects;
>   pages = oldpage->pages;
>@@ -2283,8 +2288,17 @@ static void put_cpu_partial(struct kmem_cache *s, 
>struct page *page, int drain)
>   page->pobjects = pobjects;
>   page->next = oldpage;
> 
>-  } while (this_cpu_cmpxchg(s->cpu_slab->partial, oldpage, page)
>-  != oldpage);
>+  /* we dont' change tid, but want to make sure it didn't change
>+   * in between. We don't really hope alloc/free not happen on
>+   * this CPU, but don't want the first slab be removed from and
>+   * then re-added as head to this partial list. If that case
>+   * happened, pobjects may read 0xdead when this slab is just
>+   * removed from kmem_cache_node by other CPU setting lru.prev
>+   * to LIST_POISON2.
>+   */
>+  } while (this_cpu_cmpxchg_double(s->cpu_slab->partial, s->cpu_slab->tid,
>+   oldpage, tid, page, tid) == 0);
>+
>   if (unlikely(!s->cpu_partial)) {
>   unsigned long flags;
> 
>-- 
>2.9.5

-- 
Wei Yang
Help you, Help me

Re: [PATCH v7 1/4] dt-bindings: pps: descriptor-based gpio, capture-clear addition

[tom burkart]

Quoting Rob Herring :


On Sat, Nov 17, 2018 at 4:35 AM tom burkart  wrote:


Quoting Rob Herring :

> On Wed, Nov 14, 2018 at 11:54:29PM +1100, Tom Burkart wrote:
>> This patch changes the devicetree bindings for the pps-gpio driver
>> from the integer based ABI to the descriptor based ABI.
> ? That has nothing to do with DT.

I believe it does, as the change in ABI forces a rename in the DT
naming convention.
This is due to the descriptor based ABI appending "-gpio" or "-gpios" (see
Documentation/gpio/base.txt.)
Admittedly, I may have called it by the wrong name due to ignorance,
my apologies.


If what you say is correct, then you can't change this driver. You'll
break compatibility with any existing DT.

Changing the binding reasoning should purely be that this is the
preferred form. Bindings must be independent from changing kernel
APIs.


See comments from Philip Zabel.  I misread the documentation and this  
has now been corrected in v8 of the patch.  I hope that eliminates all  
comments made above.



>>  It also adds
>> documentation for the device tree capture-clear option.  The legacy
>> device tree entry for the GPIO pin is supported.
>>
>> Signed-off-by: Tom Burkart 
>> ---
>>  Documentation/devicetree/bindings/pps/pps-gpio.txt | 8 ++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> b/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> index 3683874832ae..6c9fc0998d94 100644
>> --- a/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> +++ b/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> @@ -5,19 +5,23 @@ a GPIO pin.
>>
>>  Required properties:
>>  - compatible: should be "pps-gpio"
>> -- gpios: one PPS GPIO in the format described by ../gpio/gpio.txt
>> +- pps-gpios: one PPS GPIO in the format described by ../gpio/gpio.txt
>> +Alternatively (DEPRECATED), instead of pps-gpios above, it may have:
>> +- gpios: one PPS GPIO as above
>>
>>  Optional properties:
>>  - assert-falling-edge: when present, assert is indicated by a  
falling edge

>> (instead of by a rising edge)
>> +- capture-clear: when present, also capture the PPS clear event
>
> Is this a h/w thing? or driver configuration?

Driver configuration.  Most of the code was present in the driver, yet
it was not documented, or usable due to a two line (code) omission
(the value was not being fetched from DT).


So what determines how you want to configure this? If the user will
want to change it, then it should be a sysfs attr and exposed to
userspace. If it depends on h/w config for a board then it can be in
DT.


Sorry, I misled you somewhat.  If the PPS pulse active transition from  
the hardware is on the falling edge, this flag is required to get the  
OS to use that as the active transition.  This would not change at the  
user's whim but rather it is dependent on connected hardware.


Tom

Re: [PATCH v7 1/4] dt-bindings: pps: descriptor-based gpio, capture-clear addition

[tom burkart]

Quoting Rob Herring :


On Sat, Nov 17, 2018 at 4:35 AM tom burkart  wrote:


Quoting Rob Herring :

> On Wed, Nov 14, 2018 at 11:54:29PM +1100, Tom Burkart wrote:
>> This patch changes the devicetree bindings for the pps-gpio driver
>> from the integer based ABI to the descriptor based ABI.
> ? That has nothing to do with DT.

I believe it does, as the change in ABI forces a rename in the DT
naming convention.
This is due to the descriptor based ABI appending "-gpio" or "-gpios" (see
Documentation/gpio/base.txt.)
Admittedly, I may have called it by the wrong name due to ignorance,
my apologies.


If what you say is correct, then you can't change this driver. You'll
break compatibility with any existing DT.

Changing the binding reasoning should purely be that this is the
preferred form. Bindings must be independent from changing kernel
APIs.


See comments from Philip Zabel.  I misread the documentation and this  
has now been corrected in v8 of the patch.  I hope that eliminates all  
comments made above.



>>  It also adds
>> documentation for the device tree capture-clear option.  The legacy
>> device tree entry for the GPIO pin is supported.
>>
>> Signed-off-by: Tom Burkart 
>> ---
>>  Documentation/devicetree/bindings/pps/pps-gpio.txt | 8 ++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> b/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> index 3683874832ae..6c9fc0998d94 100644
>> --- a/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> +++ b/Documentation/devicetree/bindings/pps/pps-gpio.txt
>> @@ -5,19 +5,23 @@ a GPIO pin.
>>
>>  Required properties:
>>  - compatible: should be "pps-gpio"
>> -- gpios: one PPS GPIO in the format described by ../gpio/gpio.txt
>> +- pps-gpios: one PPS GPIO in the format described by ../gpio/gpio.txt
>> +Alternatively (DEPRECATED), instead of pps-gpios above, it may have:
>> +- gpios: one PPS GPIO as above
>>
>>  Optional properties:
>>  - assert-falling-edge: when present, assert is indicated by a  
falling edge

>> (instead of by a rising edge)
>> +- capture-clear: when present, also capture the PPS clear event
>
> Is this a h/w thing? or driver configuration?

Driver configuration.  Most of the code was present in the driver, yet
it was not documented, or usable due to a two line (code) omission
(the value was not being fetched from DT).


So what determines how you want to configure this? If the user will
want to change it, then it should be a sysfs attr and exposed to
userspace. If it depends on h/w config for a board then it can be in
DT.


Sorry, I misled you somewhat.  If the PPS pulse active transition from  
the hardware is on the falling edge, this flag is required to get the  
OS to use that as the active transition.  This would not change at the  
user's whim but rather it is dependent on connected hardware.


Tom

Re: [PATCH v5 12/18] arm64: dts: qcom: pms405: add gpios

[Bjorn Andersson]

On Fri 09 Nov 01:44 PST 2018, Vinod Koul wrote:

> Add the GPIOs present on PMS405 chip.
> 
> Signed-off-by: Vinod Koul 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> ---
>  arch/arm64/boot/dts/qcom/pms405.dtsi | 19 +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/pms405.dtsi 
> b/arch/arm64/boot/dts/qcom/pms405.dtsi
> index 2b275bbdafa3..8e5a8573430e 100644
> --- a/arch/arm64/boot/dts/qcom/pms405.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pms405.dtsi
> @@ -10,6 +10,25 @@
>   #address-cells = <1>;
>   #size-cells = <0>;
>  
> + pms405_gpios: gpio@c000 {
> + compatible = "qcom,pms405-gpio";
> + reg = <0xc000>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + interrupts = <0 0xc0 0 IRQ_TYPE_NONE>,
> + <0 0xc1 0 IRQ_TYPE_NONE>,
> + <0 0xc2 0 IRQ_TYPE_NONE>,
> + <0 0xc3 0 IRQ_TYPE_NONE>,
> + <0 0xc4 0 IRQ_TYPE_NONE>,
> + <0 0xc5 0 IRQ_TYPE_NONE>,
> + <0 0xc6 0 IRQ_TYPE_NONE>,
> + <0 0xc7 0 IRQ_TYPE_NONE>,
> + <0 0xc8 0 IRQ_TYPE_NONE>,
> + <0 0xc9 0 IRQ_TYPE_NONE>,
> + <0 0xca 0 IRQ_TYPE_NONE>,
> + <0 0xcb 0 IRQ_TYPE_NONE>;
> + };
> +
>   rtc@6000 {
>   compatible = "qcom,pm8941-rtc";
>   reg = <0x6000>;
> -- 
> 2.14.4
> 

Re: [PATCH v5 12/18] arm64: dts: qcom: pms405: add gpios

[Bjorn Andersson]

On Fri 09 Nov 01:44 PST 2018, Vinod Koul wrote:

> Add the GPIOs present on PMS405 chip.
> 
> Signed-off-by: Vinod Koul 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> ---
>  arch/arm64/boot/dts/qcom/pms405.dtsi | 19 +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/pms405.dtsi 
> b/arch/arm64/boot/dts/qcom/pms405.dtsi
> index 2b275bbdafa3..8e5a8573430e 100644
> --- a/arch/arm64/boot/dts/qcom/pms405.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pms405.dtsi
> @@ -10,6 +10,25 @@
>   #address-cells = <1>;
>   #size-cells = <0>;
>  
> + pms405_gpios: gpio@c000 {
> + compatible = "qcom,pms405-gpio";
> + reg = <0xc000>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + interrupts = <0 0xc0 0 IRQ_TYPE_NONE>,
> + <0 0xc1 0 IRQ_TYPE_NONE>,
> + <0 0xc2 0 IRQ_TYPE_NONE>,
> + <0 0xc3 0 IRQ_TYPE_NONE>,
> + <0 0xc4 0 IRQ_TYPE_NONE>,
> + <0 0xc5 0 IRQ_TYPE_NONE>,
> + <0 0xc6 0 IRQ_TYPE_NONE>,
> + <0 0xc7 0 IRQ_TYPE_NONE>,
> + <0 0xc8 0 IRQ_TYPE_NONE>,
> + <0 0xc9 0 IRQ_TYPE_NONE>,
> + <0 0xca 0 IRQ_TYPE_NONE>,
> + <0 0xcb 0 IRQ_TYPE_NONE>;
> + };
> +
>   rtc@6000 {
>   compatible = "qcom,pm8941-rtc";
>   reg = <0x6000>;
> -- 
> 2.14.4
> 

Re: [PATCH v5 02/18] arm64: dts: qcom: qcs404-evb: add dts files for EVBs

[Bjorn Andersson]

On Fri 09 Nov 01:44 PST 2018, Vinod Koul wrote:

> QCS404 has two EVBs, EVB-1000 and EVB-4000. These boards are mostly
> similar with few differences in the peripherals used.
> 
> So use a common qcs404-evb.dtsi which contains the common parts and use
> qcs404-evb-1000.dts and qcs404-evb-4000.dts for diffs
> 
> Signed-off-by: Vinod Koul 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> ---
>  arch/arm64/boot/dts/qcom/Makefile|  2 ++
>  arch/arm64/boot/dts/qcom/qcs404-evb-1000.dts | 11 +++
>  arch/arm64/boot/dts/qcom/qcs404-evb-4000.dts | 11 +++
>  arch/arm64/boot/dts/qcom/qcs404-evb.dtsi | 14 ++
>  4 files changed, 38 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/qcs404-evb-1000.dts
>  create mode 100644 arch/arm64/boot/dts/qcom/qcs404-evb-4000.dts
>  create mode 100644 arch/arm64/boot/dts/qcom/qcs404-evb.dtsi
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile 
> b/arch/arm64/boot/dts/qcom/Makefile
> index a658c07652a7..21d548f02d39 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -8,3 +8,5 @@ dtb-$(CONFIG_ARCH_QCOM)   += msm8994-angler-rev-101.dtb
>  dtb-$(CONFIG_ARCH_QCOM)  += msm8996-mtp.dtb
>  dtb-$(CONFIG_ARCH_QCOM)  += msm8998-mtp.dtb
>  dtb-$(CONFIG_ARCH_QCOM)  += sdm845-mtp.dtb
> +dtb-$(CONFIG_ARCH_QCOM)  += qcs404-evb-1000.dtb
> +dtb-$(CONFIG_ARCH_QCOM)  += qcs404-evb-4000.dtb
> diff --git a/arch/arm64/boot/dts/qcom/qcs404-evb-1000.dts 
> b/arch/arm64/boot/dts/qcom/qcs404-evb-1000.dts
> new file mode 100644
> index ..2c14903d808e
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/qcs404-evb-1000.dts
> @@ -0,0 +1,11 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018, Linaro Limited
> +
> +/dts-v1/;
> +
> +#include "qcs404-evb.dtsi"
> +
> +/ {
> + model = "Qualcomm Technologies, Inc. QCS404 EVB 1000";
> + compatible = "qcom,qcs404-evb";
> +};
> diff --git a/arch/arm64/boot/dts/qcom/qcs404-evb-4000.dts 
> b/arch/arm64/boot/dts/qcom/qcs404-evb-4000.dts
> new file mode 100644
> index ..11269ad3de0d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/qcs404-evb-4000.dts
> @@ -0,0 +1,11 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018, Linaro Limited
> +
> +/dts-v1/;
> +
> +#include "qcs404-evb.dtsi"
> +
> +/ {
> + model = "Qualcomm Technologies, Inc. QCS404 EVB 4000";
> + compatible = "qcom,qcs404-evb";
> +};
> diff --git a/arch/arm64/boot/dts/qcom/qcs404-evb.dtsi 
> b/arch/arm64/boot/dts/qcom/qcs404-evb.dtsi
> new file mode 100644
> index ..91ecbdf0ecda
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/qcs404-evb.dtsi
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018, Linaro Limited
> +
> +#include "qcs404.dtsi"
> +
> +/ {
> + aliases {
> + serial0 = _uart2;
> + };
> +
> + chosen {
> + stdout-path = "serial0";
> + };
> +};
> -- 
> 2.14.4
> 

Re: [PATCH v5 02/18] arm64: dts: qcom: qcs404-evb: add dts files for EVBs

[Bjorn Andersson]

On Fri 09 Nov 01:44 PST 2018, Vinod Koul wrote:

> QCS404 has two EVBs, EVB-1000 and EVB-4000. These boards are mostly
> similar with few differences in the peripherals used.
> 
> So use a common qcs404-evb.dtsi which contains the common parts and use
> qcs404-evb-1000.dts and qcs404-evb-4000.dts for diffs
> 
> Signed-off-by: Vinod Koul 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> ---
>  arch/arm64/boot/dts/qcom/Makefile|  2 ++
>  arch/arm64/boot/dts/qcom/qcs404-evb-1000.dts | 11 +++
>  arch/arm64/boot/dts/qcom/qcs404-evb-4000.dts | 11 +++
>  arch/arm64/boot/dts/qcom/qcs404-evb.dtsi | 14 ++
>  4 files changed, 38 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/qcs404-evb-1000.dts
>  create mode 100644 arch/arm64/boot/dts/qcom/qcs404-evb-4000.dts
>  create mode 100644 arch/arm64/boot/dts/qcom/qcs404-evb.dtsi
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile 
> b/arch/arm64/boot/dts/qcom/Makefile
> index a658c07652a7..21d548f02d39 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -8,3 +8,5 @@ dtb-$(CONFIG_ARCH_QCOM)   += msm8994-angler-rev-101.dtb
>  dtb-$(CONFIG_ARCH_QCOM)  += msm8996-mtp.dtb
>  dtb-$(CONFIG_ARCH_QCOM)  += msm8998-mtp.dtb
>  dtb-$(CONFIG_ARCH_QCOM)  += sdm845-mtp.dtb
> +dtb-$(CONFIG_ARCH_QCOM)  += qcs404-evb-1000.dtb
> +dtb-$(CONFIG_ARCH_QCOM)  += qcs404-evb-4000.dtb
> diff --git a/arch/arm64/boot/dts/qcom/qcs404-evb-1000.dts 
> b/arch/arm64/boot/dts/qcom/qcs404-evb-1000.dts
> new file mode 100644
> index ..2c14903d808e
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/qcs404-evb-1000.dts
> @@ -0,0 +1,11 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018, Linaro Limited
> +
> +/dts-v1/;
> +
> +#include "qcs404-evb.dtsi"
> +
> +/ {
> + model = "Qualcomm Technologies, Inc. QCS404 EVB 1000";
> + compatible = "qcom,qcs404-evb";
> +};
> diff --git a/arch/arm64/boot/dts/qcom/qcs404-evb-4000.dts 
> b/arch/arm64/boot/dts/qcom/qcs404-evb-4000.dts
> new file mode 100644
> index ..11269ad3de0d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/qcs404-evb-4000.dts
> @@ -0,0 +1,11 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018, Linaro Limited
> +
> +/dts-v1/;
> +
> +#include "qcs404-evb.dtsi"
> +
> +/ {
> + model = "Qualcomm Technologies, Inc. QCS404 EVB 4000";
> + compatible = "qcom,qcs404-evb";
> +};
> diff --git a/arch/arm64/boot/dts/qcom/qcs404-evb.dtsi 
> b/arch/arm64/boot/dts/qcom/qcs404-evb.dtsi
> new file mode 100644
> index ..91ecbdf0ecda
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/qcs404-evb.dtsi
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018, Linaro Limited
> +
> +#include "qcs404.dtsi"
> +
> +/ {
> + aliases {
> + serial0 = _uart2;
> + };
> +
> + chosen {
> + stdout-path = "serial0";
> + };
> +};
> -- 
> 2.14.4
> 

Re: [PATCH v5 01/18] arm64: dts: qcom: qcs404: add base dts files

[Bjorn Andersson]

On Fri 09 Nov 01:44 PST 2018, Vinod Koul wrote:

> Add base dts files for QCS404 chipset along with cpu, timer,
> gcc and uart2 nodes.
> 
> Signed-off-by: Vinod Koul 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> ---
>  arch/arm64/boot/dts/qcom/qcs404.dtsi | 175 
> +++
>  1 file changed, 175 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/qcs404.dtsi
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi 
> b/arch/arm64/boot/dts/qcom/qcs404.dtsi
> new file mode 100644
> index ..91abcdc78505
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
> @@ -0,0 +1,175 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018, Linaro Limited
> +
> +#include 
> +#include 
> +
> +/ {
> + interrupt-parent = <>;
> +
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + chosen { };
> +
> + clocks {
> + xo_board: xo-board {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <1920>;
> + };
> + };
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + CPU0: cpu@100 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x100>;
> + enable-method = "psci";
> + next-level-cache = <_0>;
> + };
> +
> + CPU1: cpu@101 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x101>;
> + enable-method = "psci";
> + next-level-cache = <_0>;
> + };
> +
> + CPU2: cpu@102 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x102>;
> + enable-method = "psci";
> + next-level-cache = <_0>;
> + };
> +
> + CPU3: cpu@103 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x103>;
> + enable-method = "psci";
> + next-level-cache = <_0>;
> + };
> +
> + L2_0: l2-cache {
> + compatible = "cache";
> + cache-level = <2>;
> + };
> + };
> +
> + memory@8000 {
> + device_type = "memory";
> + /* We expect the bootloader to fill in the size */
> + reg = <0 0x8000 0 0>;
> + };
> +
> + psci {
> + compatible = "arm,psci-1.0";
> + method = "smc";
> + };
> +
> + soc: soc@0 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0 0 0x>;
> + compatible = "simple-bus";
> +
> + gcc: clock-controller@180 {
> + compatible = "qcom,gcc-qcs404";
> + reg = <0x0180 0x8>;
> + #clock-cells = <1>;
> +
> + assigned-clocks = < GCC_APSS_AHB_CLK_SRC>;
> + assigned-clock-rates = <1920>;
> + };
> +
> + blsp1_uart2: serial@78b1000 {
> + compatible = "qcom,msm-uartdm-v1.4", "qcom,msm-uartdm";
> + reg = <0x078b1000 0x200>;
> + interrupts = ;
> + clocks = < GCC_BLSP1_UART2_APPS_CLK>, < 
> GCC_BLSP1_AHB_CLK>;
> + clock-names = "core", "iface";
> + status = "okay";
> + };
> +
> + intc: interrupt-controller@b00 {
> + compatible = "qcom,msm-qgic2";
> + interrupt-controller;
> + #interrupt-cells = <3>;
> + reg = <0x0b00 0x1000>,
> +   <0x0b002000 0x1000>;
> + };
> +
> + timer@b12 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + compatible = "arm,armv7-timer-mem";
> + reg = <0x0b12 0x1000>;
> + clock-frequency = <1920>;
> +
> + frame@b121000 {
> + frame-number = <0>;
> + interrupts = ,
> +  ;
> + reg = <0x0b121000 0x1000>,
> +   <0x0b122000 0x1000>;
> + };
> +
> + frame@b123000 {
> + frame-number = <1>;
> + interrupts = ;
> + reg = <0x0b123000 0x1000>;
> +

Re: [PATCH v5 01/18] arm64: dts: qcom: qcs404: add base dts files

[Bjorn Andersson]

On Fri 09 Nov 01:44 PST 2018, Vinod Koul wrote:

> Add base dts files for QCS404 chipset along with cpu, timer,
> gcc and uart2 nodes.
> 
> Signed-off-by: Vinod Koul 

Reviewed-by: Bjorn Andersson 

Regards,
Bjorn

> ---
>  arch/arm64/boot/dts/qcom/qcs404.dtsi | 175 
> +++
>  1 file changed, 175 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/qcs404.dtsi
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi 
> b/arch/arm64/boot/dts/qcom/qcs404.dtsi
> new file mode 100644
> index ..91abcdc78505
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
> @@ -0,0 +1,175 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018, Linaro Limited
> +
> +#include 
> +#include 
> +
> +/ {
> + interrupt-parent = <>;
> +
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + chosen { };
> +
> + clocks {
> + xo_board: xo-board {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <1920>;
> + };
> + };
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + CPU0: cpu@100 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x100>;
> + enable-method = "psci";
> + next-level-cache = <_0>;
> + };
> +
> + CPU1: cpu@101 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x101>;
> + enable-method = "psci";
> + next-level-cache = <_0>;
> + };
> +
> + CPU2: cpu@102 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x102>;
> + enable-method = "psci";
> + next-level-cache = <_0>;
> + };
> +
> + CPU3: cpu@103 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x103>;
> + enable-method = "psci";
> + next-level-cache = <_0>;
> + };
> +
> + L2_0: l2-cache {
> + compatible = "cache";
> + cache-level = <2>;
> + };
> + };
> +
> + memory@8000 {
> + device_type = "memory";
> + /* We expect the bootloader to fill in the size */
> + reg = <0 0x8000 0 0>;
> + };
> +
> + psci {
> + compatible = "arm,psci-1.0";
> + method = "smc";
> + };
> +
> + soc: soc@0 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0 0 0x>;
> + compatible = "simple-bus";
> +
> + gcc: clock-controller@180 {
> + compatible = "qcom,gcc-qcs404";
> + reg = <0x0180 0x8>;
> + #clock-cells = <1>;
> +
> + assigned-clocks = < GCC_APSS_AHB_CLK_SRC>;
> + assigned-clock-rates = <1920>;
> + };
> +
> + blsp1_uart2: serial@78b1000 {
> + compatible = "qcom,msm-uartdm-v1.4", "qcom,msm-uartdm";
> + reg = <0x078b1000 0x200>;
> + interrupts = ;
> + clocks = < GCC_BLSP1_UART2_APPS_CLK>, < 
> GCC_BLSP1_AHB_CLK>;
> + clock-names = "core", "iface";
> + status = "okay";
> + };
> +
> + intc: interrupt-controller@b00 {
> + compatible = "qcom,msm-qgic2";
> + interrupt-controller;
> + #interrupt-cells = <3>;
> + reg = <0x0b00 0x1000>,
> +   <0x0b002000 0x1000>;
> + };
> +
> + timer@b12 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + compatible = "arm,armv7-timer-mem";
> + reg = <0x0b12 0x1000>;
> + clock-frequency = <1920>;
> +
> + frame@b121000 {
> + frame-number = <0>;
> + interrupts = ,
> +  ;
> + reg = <0x0b121000 0x1000>,
> +   <0x0b122000 0x1000>;
> + };
> +
> + frame@b123000 {
> + frame-number = <1>;
> + interrupts = ;
> + reg = <0x0b123000 0x1000>;
> +

Re: [PATCH] mtd: rawnand: qcom: Namespace prefix some commands

[Miquel Raynal]

Hi Olof,

Olof Johansson  wrote on Fri, 16 Nov 2018 19:43:27
-0800:

> PAGE_READ is used by RISC-V arch code included through mm headers,
> and it makes sense to bring in a prefix on these in the driver.
> 
> drivers/mtd/nand/raw/qcom_nandc.c:153: warning: "PAGE_READ" redefined
>  #define PAGE_READ   0x2
> In file included from include/linux/memremap.h:7,
>  from include/linux/mm.h:27,
>  from include/linux/scatterlist.h:8,
>  from include/linux/dma-mapping.h:11,
>  from drivers/mtd/nand/raw/qcom_nandc.c:17:
> arch/riscv/include/asm/pgtable.h:48: note: this is the location of the 
> previous definition
> 
> Caught by riscv allmodconfig.
> 
> Signed-off-by: Olof Johansson 
> ---

Reviewed-by: Miquel Raynal 

Boris, do you want to queue this as a fix?

Thanks,
Miquèl

Re: [PATCH] mtd: rawnand: qcom: Namespace prefix some commands

[Miquel Raynal]

Hi Olof,

Olof Johansson  wrote on Fri, 16 Nov 2018 19:43:27
-0800:

> PAGE_READ is used by RISC-V arch code included through mm headers,
> and it makes sense to bring in a prefix on these in the driver.
> 
> drivers/mtd/nand/raw/qcom_nandc.c:153: warning: "PAGE_READ" redefined
>  #define PAGE_READ   0x2
> In file included from include/linux/memremap.h:7,
>  from include/linux/mm.h:27,
>  from include/linux/scatterlist.h:8,
>  from include/linux/dma-mapping.h:11,
>  from drivers/mtd/nand/raw/qcom_nandc.c:17:
> arch/riscv/include/asm/pgtable.h:48: note: this is the location of the 
> previous definition
> 
> Caught by riscv allmodconfig.
> 
> Signed-off-by: Olof Johansson 
> ---

Reviewed-by: Miquel Raynal 

Boris, do you want to queue this as a fix?

Thanks,
Miquèl

Re: [PATCH v6 1/3] staging: mt7621-mmc: Remove #if 0 blocks in sd.c

[NeilBrown]

On Thu, Oct 04 2018, Nishad Kamdar wrote:

> This patch removes #if 0 code blocks and usages of
> functions defined in the #if 0 blocks in sd.c.
>
> Signed-off-by: Nishad Kamdar 

Hi Nishad,
 thanks for this patch (and others) and apologies for not
 reviewing/testing it earlier.
 Unfortunately there is a problem - see below.
 
>  
> -#if 0 /* --- by chhung */
> -/* For E2 only */
> -static u8 clk_src_bit[4] = {
> - 0, 3, 5, 7
> -};
> -
>  static void msdc_select_clksrc(struct msdc_host *host, unsigned char clksrc)
>  {
>   u32 val;

Above you removed the first few lines of a "#if 0" block, but not all of
it.  Previously the function msdc_select_clksrc() was not compiled at
all.  Now it is, and that causes an error - MSDC_CLKSRC_REG is not
defined.

Would you be able to send a follow-patch patch which removes this
function and the "#endif /* end of --- */" at the end?

Thanks,
NeilBrown


signature.asc
Description: PGP signature

Re: [PATCH v6 1/3] staging: mt7621-mmc: Remove #if 0 blocks in sd.c

[NeilBrown]

On Thu, Oct 04 2018, Nishad Kamdar wrote:

> This patch removes #if 0 code blocks and usages of
> functions defined in the #if 0 blocks in sd.c.
>
> Signed-off-by: Nishad Kamdar 

Hi Nishad,
 thanks for this patch (and others) and apologies for not
 reviewing/testing it earlier.
 Unfortunately there is a problem - see below.
 
>  
> -#if 0 /* --- by chhung */
> -/* For E2 only */
> -static u8 clk_src_bit[4] = {
> - 0, 3, 5, 7
> -};
> -
>  static void msdc_select_clksrc(struct msdc_host *host, unsigned char clksrc)
>  {
>   u32 val;

Above you removed the first few lines of a "#if 0" block, but not all of
it.  Previously the function msdc_select_clksrc() was not compiled at
all.  Now it is, and that causes an error - MSDC_CLKSRC_REG is not
defined.

Would you be able to send a follow-patch patch which removes this
function and the "#endif /* end of --- */" at the end?

Thanks,
NeilBrown


signature.asc
Description: PGP signature

[of] deadlock b/c of gpiochip and pinctrl dependency

[Robert Abel]

Hi!

I found a dependency issue with gpiochip and pinctrl while using the gpio-hog 
feature that was implemented by Benoit
Parrot in f625d4601759f. Relevant device-tree overlay shown below [3].

I ran into a deadlock situation, because adding a gpiochip requires a 
registered pinctrl with registered ranges, because
the function to add the gpiochip will check for hogged pins and request gpio 
pin functionality for them.
This works in situations when gpiochip and pinctrl functionality don't depend 
on each other. However, consider the
following code, which is from the bcm2835 driver for Raspberry Pi found here 
[1]:

pinctrl-bcm2835.c:
err = gpiochip_add_data(>gpio_chip, pc);
if (err) {
dev_err(dev, "could not add GPIO chipn");
return err;
}

...

pc->pctl_dev = devm_pinctrl_register(dev, _pinctrl_desc, pc);
if (IS_ERR(pc->pctl_dev)) {
gpiochip_remove(>gpio_chip);
return PTR_ERR(pc->pctl_dev);
}

...

pinctrl_add_gpio_range(pc->pctl_dev, >gpio_range);


It's not very obvious, that calling gpiochip_add_data should only be done after 
adding the required ranges to pinctrl,
because the following happens:

gpiochip_add_data(_with_key):
  of_gpiochip_add:
of_gpiochip_scan_gpios:
  for_each_available_child_of_node(chip->of_node, np) with "gpio-hog":
gpiod_hog
  gpiochip_request_own_desc:
gpiod_request_commit:
  status = chip->request(chip, gpio_chip_hwgpio(desc)):
gpiochip_generic_request(...):
  pinctrl_gpio_request:
pinctrl_get_device_gpio_range:
  /* GPIO range not found, because it wasn't registered yet 
*/
  return -EPROBE_DEFER;

This of course deadlocks my system, because everything ends up waiting on the 
gpiochip, which cannot ever be
instantiated. I couldn't find any documentation explicitly mentioning this 
dependency between gpiochip and pinctrl.
My workaround for the moment is to re-order the gpiochip_add_data call after 
the devm_pinctrl_register and
pinctrl_add_gpio_range calls. I haven't observed any ill effect, but I'm not 
sure what other components may already act
on pinctrl while gpiochip is dangling or if any of the other functions that are 
called by add require it as well.

Obviously, this approach won't work for SoCs where setting certain pinctrl 
settings need to touch gpio settings to e.g.
activate pull-ups.

In general, I sought the same functionality that Markus Pargmann was trying to 
implement in 2015 [2].
That is, to name and possibly export gpios through device tree.

It seems disadvantageous that one cannot currently specify pinctrl settings 
while hogging or export by name.
Dynamic device tree overlays also don't work. Perhaps an actual driver instead 
of the hogging mechanism would solve all
dependency issues better?

Regards,

Robert

Please CC, as I'm off-list.


  [1]: 
https://github.com/raspberrypi/linux/blob/83ea2e93/drivers/pinctrl/bcm/pinctrl-bcm2835.c#L1027
  [2]: 
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/357418.html
  [3]: gpio-hog.dtso
   /dts-v1/;
   /plugin/;

   #include 

   / {
   compatible = "brcm,bcm2708";
   fragment@0 {
   target = <>;
   __overlay__ {
   myLed {
   gpios = <18 GPIO_ACTIVE_HIGH>;
   gpio-hog;
   output-low;
   };
   };
   };
   };

[of] deadlock b/c of gpiochip and pinctrl dependency

[Robert Abel]

Hi!

I found a dependency issue with gpiochip and pinctrl while using the gpio-hog 
feature that was implemented by Benoit
Parrot in f625d4601759f. Relevant device-tree overlay shown below [3].

I ran into a deadlock situation, because adding a gpiochip requires a 
registered pinctrl with registered ranges, because
the function to add the gpiochip will check for hogged pins and request gpio 
pin functionality for them.
This works in situations when gpiochip and pinctrl functionality don't depend 
on each other. However, consider the
following code, which is from the bcm2835 driver for Raspberry Pi found here 
[1]:

pinctrl-bcm2835.c:
err = gpiochip_add_data(>gpio_chip, pc);
if (err) {
dev_err(dev, "could not add GPIO chipn");
return err;
}

...

pc->pctl_dev = devm_pinctrl_register(dev, _pinctrl_desc, pc);
if (IS_ERR(pc->pctl_dev)) {
gpiochip_remove(>gpio_chip);
return PTR_ERR(pc->pctl_dev);
}

...

pinctrl_add_gpio_range(pc->pctl_dev, >gpio_range);


It's not very obvious, that calling gpiochip_add_data should only be done after 
adding the required ranges to pinctrl,
because the following happens:

gpiochip_add_data(_with_key):
  of_gpiochip_add:
of_gpiochip_scan_gpios:
  for_each_available_child_of_node(chip->of_node, np) with "gpio-hog":
gpiod_hog
  gpiochip_request_own_desc:
gpiod_request_commit:
  status = chip->request(chip, gpio_chip_hwgpio(desc)):
gpiochip_generic_request(...):
  pinctrl_gpio_request:
pinctrl_get_device_gpio_range:
  /* GPIO range not found, because it wasn't registered yet 
*/
  return -EPROBE_DEFER;

This of course deadlocks my system, because everything ends up waiting on the 
gpiochip, which cannot ever be
instantiated. I couldn't find any documentation explicitly mentioning this 
dependency between gpiochip and pinctrl.
My workaround for the moment is to re-order the gpiochip_add_data call after 
the devm_pinctrl_register and
pinctrl_add_gpio_range calls. I haven't observed any ill effect, but I'm not 
sure what other components may already act
on pinctrl while gpiochip is dangling or if any of the other functions that are 
called by add require it as well.

Obviously, this approach won't work for SoCs where setting certain pinctrl 
settings need to touch gpio settings to e.g.
activate pull-ups.

In general, I sought the same functionality that Markus Pargmann was trying to 
implement in 2015 [2].
That is, to name and possibly export gpios through device tree.

It seems disadvantageous that one cannot currently specify pinctrl settings 
while hogging or export by name.
Dynamic device tree overlays also don't work. Perhaps an actual driver instead 
of the hogging mechanism would solve all
dependency issues better?

Regards,

Robert

Please CC, as I'm off-list.


  [1]: 
https://github.com/raspberrypi/linux/blob/83ea2e93/drivers/pinctrl/bcm/pinctrl-bcm2835.c#L1027
  [2]: 
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/357418.html
  [3]: gpio-hog.dtso
   /dts-v1/;
   /plugin/;

   #include 

   / {
   compatible = "brcm,bcm2708";
   fragment@0 {
   target = <>;
   __overlay__ {
   myLed {
   gpios = <18 GPIO_ACTIVE_HIGH>;
   gpio-hog;
   output-low;
   };
   };
   };
   };

linux-next: Unable to mount a cgroup file system

[Andrei Vagin]

Hello,

We run CRIU tests on linux-next kernels. Today, I found that our test
robot hangs up on mounting a cgroup file system.

https://travis-ci.org/avagin/linux/jobs/455732006

  632 ?Ssl0:00 /usr/bin/containerd
  843 ?Sl 0:00  \_ containerd-shim -namespace moby
-workdir 
/var/lib/containerd/io.containerd.runtime.v1.linux/moby/c2311352d53eed1f5094580102e41c2a02eaf98b8626c86ccf314599101b26
  862 pts/0Ss+0:00  \_ python test/zdtm.py run -T
zdtm/static/cgroup.*
 1652 pts/0S+ 0:00  \_ flock zdtm_mount_cgroups.lock
./zdtm_umount_cgroups
 1653 pts/0S+ 0:00  \_ /bin/sh ./zdtm_umount_cgroups
 1659 pts/0D+ 0:06  \_ mount -t cgroup -o
none,name=zdtmtst.defaultroot zdtm zdtm.9QFGko

[root@fc24 ~]# cat /proc/1659/stack
[<0>] msleep+0x38/0x40
[<0>] cgroup1_get_tree+0x4e1/0x749
[<0>] vfs_get_tree+0x5e/0x140
[<0>] do_mount+0x326/0xc70
[<0>] ksys_mount+0xba/0xd0
[<0>] __x64_sys_mount+0x21/0x30
[<0>] do_syscall_64+0x60/0x210
[<0>] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[<0>] 0x

[root@fc24 ~]# cat /proc/1659/cgroup  | grep zdtm
13:name=zdtmtst.defaultroot:/
12:name=zdtmtst:/

[root@fc24 ~]# strace -fp 1659
strace: Process 1659 attached
mount("zdtm", "/criu/test/zdtm.9QFGko", "cgroup", MS_MGC_VAL,
"none,name=zdtmtst.defaultroot") = ? ERESTARTNOINTR (To be restarted)
mount("zdtm", "/criu/test/zdtm.9QFGko", "cgroup", MS_MGC_VAL,
"none,name=zdtmtst.defaultroot") = ? ERESTARTNOINTR (To be restarted)
mount("zdtm", "/criu/test/zdtm.9QFGko", "cgroup", MS_MGC_VAL,
"none,name=zdtmtst.defaultroot") = ? ERESTARTNOINTR (To be restarted)
mount("zdtm", "/criu/test/zdtm.9QFGko", "cgroup", MS_MGC_VAL,
"none,name=zdtmtst.defaultroot") = ? ERESTARTNOINTR (To be restarted)
mount("zdtm", "/criu/test/zdtm.9QFGko", "cgroup", MS_MGC_VAL,
"none,name=zdtmtst.defaultroot") = ? ERESTARTNOINTR (To be restarted)

Steps to reproduce:
I don't know how to reproduce this issue without running criu tests. I
tried to create a simple reproducer, but I failed. So I created a
docker container and the problem can be reproduced by running this
command:
docker run --rm -it --privileged -v /lib/modules:/lib/modules --tmpfs
/run docker.io/avagin/criu-fc29-cgroup python test/zdtm.py run -T
'zdtm/static/cgroup.*'

I found that something wrong is in
[16ec1a5d58ea67ba737d3a66efe9e53c6bb149f7] kernfs, sysfs, cgroup,
intel_rdt: Support fs_context

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20181116=16ec1a5d58ea67ba737d3a66efe9e53c6bb149f7

Thanks,
Andrei

linux-next: Unable to mount a cgroup file system

[Andrei Vagin]

Hello,

We run CRIU tests on linux-next kernels. Today, I found that our test
robot hangs up on mounting a cgroup file system.

https://travis-ci.org/avagin/linux/jobs/455732006

  632 ?Ssl0:00 /usr/bin/containerd
  843 ?Sl 0:00  \_ containerd-shim -namespace moby
-workdir 
/var/lib/containerd/io.containerd.runtime.v1.linux/moby/c2311352d53eed1f5094580102e41c2a02eaf98b8626c86ccf314599101b26
  862 pts/0Ss+0:00  \_ python test/zdtm.py run -T
zdtm/static/cgroup.*
 1652 pts/0S+ 0:00  \_ flock zdtm_mount_cgroups.lock
./zdtm_umount_cgroups
 1653 pts/0S+ 0:00  \_ /bin/sh ./zdtm_umount_cgroups
 1659 pts/0D+ 0:06  \_ mount -t cgroup -o
none,name=zdtmtst.defaultroot zdtm zdtm.9QFGko

[root@fc24 ~]# cat /proc/1659/stack
[<0>] msleep+0x38/0x40
[<0>] cgroup1_get_tree+0x4e1/0x749
[<0>] vfs_get_tree+0x5e/0x140
[<0>] do_mount+0x326/0xc70
[<0>] ksys_mount+0xba/0xd0
[<0>] __x64_sys_mount+0x21/0x30
[<0>] do_syscall_64+0x60/0x210
[<0>] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[<0>] 0x

[root@fc24 ~]# cat /proc/1659/cgroup  | grep zdtm
13:name=zdtmtst.defaultroot:/
12:name=zdtmtst:/

[root@fc24 ~]# strace -fp 1659
strace: Process 1659 attached
mount("zdtm", "/criu/test/zdtm.9QFGko", "cgroup", MS_MGC_VAL,
"none,name=zdtmtst.defaultroot") = ? ERESTARTNOINTR (To be restarted)
mount("zdtm", "/criu/test/zdtm.9QFGko", "cgroup", MS_MGC_VAL,
"none,name=zdtmtst.defaultroot") = ? ERESTARTNOINTR (To be restarted)
mount("zdtm", "/criu/test/zdtm.9QFGko", "cgroup", MS_MGC_VAL,
"none,name=zdtmtst.defaultroot") = ? ERESTARTNOINTR (To be restarted)
mount("zdtm", "/criu/test/zdtm.9QFGko", "cgroup", MS_MGC_VAL,
"none,name=zdtmtst.defaultroot") = ? ERESTARTNOINTR (To be restarted)
mount("zdtm", "/criu/test/zdtm.9QFGko", "cgroup", MS_MGC_VAL,
"none,name=zdtmtst.defaultroot") = ? ERESTARTNOINTR (To be restarted)

Steps to reproduce:
I don't know how to reproduce this issue without running criu tests. I
tried to create a simple reproducer, but I failed. So I created a
docker container and the problem can be reproduced by running this
command:
docker run --rm -it --privileged -v /lib/modules:/lib/modules --tmpfs
/run docker.io/avagin/criu-fc29-cgroup python test/zdtm.py run -T
'zdtm/static/cgroup.*'

I found that something wrong is in
[16ec1a5d58ea67ba737d3a66efe9e53c6bb149f7] kernfs, sysfs, cgroup,
intel_rdt: Support fs_context

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20181116=16ec1a5d58ea67ba737d3a66efe9e53c6bb149f7

Thanks,
Andrei

Re: [PATCH v2 4/4] clk: meson-gxbb: Add video clocks

[Martin Blumenstingl]

 Hi Neil,

On Tue, Nov 6, 2018 at 3:59 PM Neil Armstrong  wrote:
>
> Add the clocks entries used in the video clock path, the clock path
> is doubled to permit having different synchronized clocks for different
> parts of the video pipeline.
>
> All dividers are flagged with CLK_GET_RATE_NOCACHE, and all gates are flagged
> with CLK_IGNORE_UNUSED since they are currently directly handled by the
> Meson DRM Driver.
> Once the DRM Driver is fully migrated to using the Common Clock Framework
> to handle the video clock tree, the CLK_GET_RATE_NOCACHE and CLK_IGNORE_UNUSED
> will be dropped.
>
> Signed-off-by: Neil Armstrong 
I'm aware that this has been applied already
I only found a few nit-picks - so this is looking good!

> ---
>  drivers/clk/meson/gxbb.c | 722 
> +++
>  1 file changed, 722 insertions(+)
>
> diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
> index 0fd354b..30fbf8f 100644
> --- a/drivers/clk/meson/gxbb.c
> +++ b/drivers/clk/meson/gxbb.c
> @@ -1558,6 +1558,616 @@ static struct clk_regmap gxbb_vapb = {
> },
>  };
>
> +/* Video Clocks */
> +
> +static struct clk_regmap gxbb_vid_pll_div = {
> +   .data = &(struct meson_vid_pll_div_data){
> +   .val = {
> +   .reg_off = HHI_VID_PLL_CLK_DIV,
> +   .shift   = 0,
> +   .width   = 15,
> +   },
> +   .sel = {
> +   .reg_off = HHI_VID_PLL_CLK_DIV,
> +   .shift   = 16,
> +   .width   = 2,
> +   },
> +   },
> +   .hw.init = &(struct clk_init_data) {
> +   .name = "vid_pll_div",
> +   .ops = _vid_pll_div_ro_ops,
> +   .parent_names = (const char *[]){ "hdmi_pll" },
> +   .num_parents = 1,
> +   .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
> +   },
> +};
> +
> +const char *gxbb_vid_pll_parent_names[] = { "vid_pll_div", "hdmi_pll" };
> +
> +static struct clk_regmap gxbb_vid_pll_sel = {
> +   .data = &(struct clk_regmap_mux_data){
> +   .offset = HHI_VID_PLL_CLK_DIV,
> +   .mask = 0x1,
> +   .shift = 18,
> +   },
> +   .hw.init = &(struct clk_init_data){
> +   .name = "vid_pll_sel",
> +   .ops = _regmap_mux_ops,
> +   /*
> +* bit 18 selects from 2 possible parents:
> +* vid_pll_div or hdmi_pll
> +*/
> +   .parent_names = gxbb_vid_pll_parent_names,
> +   .num_parents = ARRAY_SIZE(gxbb_vid_pll_parent_names),
you could get rid of the comment if you inline
gxbb_vid_pll_parent_names and set num_parents to 2 manually

> +   .flags = CLK_SET_RATE_NO_REPARENT | CLK_GET_RATE_NOCACHE,
> +   },
> +};
> +
> +static struct clk_regmap gxbb_vid_pll = {
> +   .data = &(struct clk_regmap_gate_data){
> +   .offset = HHI_VID_PLL_CLK_DIV,
> +   .bit_idx = 19,
> +   },
> +   .hw.init = &(struct clk_init_data) {
> +   .name = "vid_pll",
> +   .ops = _regmap_gate_ops,
> +   .parent_names = (const char *[]){ "vid_pll_sel" },
> +   .num_parents = 1,
> +   .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> +   },
> +};
> +
> +const char *gxbb_vclk_parent_names[] = {
> +   "vid_pll", "fclk_div4", "fclk_div3", "fclk_div5", "vid_pll",
> +   "fclk_div7", "mpll1",
> +};
> +
> +static struct clk_regmap gxbb_vclk_sel = {
> +   .data = &(struct clk_regmap_mux_data){
> +   .offset = HHI_VID_CLK_CNTL,
> +   .mask = 0x7,
> +   .shift = 16,
> +   },
> +   .hw.init = &(struct clk_init_data){
> +   .name = "vclk_sel",
> +   .ops = _regmap_mux_ops,
> +   /*
> +* bits 16:18 selects from 8 possible parents:
> +* vid_pll, fclk_div4, fclk_div3, fclk_div5,
> +* vid_pll, fclk_div7, mp1
> +*/
does it select from 7 or 8 parents? you are listing only 7
the same applies for the usages below

> +   .parent_names = gxbb_vclk_parent_names,
> +   .num_parents = ARRAY_SIZE(gxbb_vclk_parent_names),
> +   .flags = CLK_SET_RATE_NO_REPARENT | CLK_GET_RATE_NOCACHE,
> +   },
> +};
> +
> +static struct clk_regmap gxbb_vclk2_sel = {
> +   .data = &(struct clk_regmap_mux_data){
> +   .offset = HHI_VIID_CLK_CNTL,
> +   .mask = 0x7,
> +   .shift = 16,
> +   },
> +   .hw.init = &(struct clk_init_data){
> +   .name = "vclk2_sel",
> +   .ops = _regmap_mux_ops,
> +   /*
> +* bits 16:18 selects from 8 possible parents:
> +* vid_pll, fclk_div4, fclk_div3, fclk_div5,
> +* vid_pll, fclk_div7, mp1
> +*/
> +   

Re: [PATCH v2 4/4] clk: meson-gxbb: Add video clocks

[Martin Blumenstingl]

 Hi Neil,

On Tue, Nov 6, 2018 at 3:59 PM Neil Armstrong  wrote:
>
> Add the clocks entries used in the video clock path, the clock path
> is doubled to permit having different synchronized clocks for different
> parts of the video pipeline.
>
> All dividers are flagged with CLK_GET_RATE_NOCACHE, and all gates are flagged
> with CLK_IGNORE_UNUSED since they are currently directly handled by the
> Meson DRM Driver.
> Once the DRM Driver is fully migrated to using the Common Clock Framework
> to handle the video clock tree, the CLK_GET_RATE_NOCACHE and CLK_IGNORE_UNUSED
> will be dropped.
>
> Signed-off-by: Neil Armstrong 
I'm aware that this has been applied already
I only found a few nit-picks - so this is looking good!

> ---
>  drivers/clk/meson/gxbb.c | 722 
> +++
>  1 file changed, 722 insertions(+)
>
> diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
> index 0fd354b..30fbf8f 100644
> --- a/drivers/clk/meson/gxbb.c
> +++ b/drivers/clk/meson/gxbb.c
> @@ -1558,6 +1558,616 @@ static struct clk_regmap gxbb_vapb = {
> },
>  };
>
> +/* Video Clocks */
> +
> +static struct clk_regmap gxbb_vid_pll_div = {
> +   .data = &(struct meson_vid_pll_div_data){
> +   .val = {
> +   .reg_off = HHI_VID_PLL_CLK_DIV,
> +   .shift   = 0,
> +   .width   = 15,
> +   },
> +   .sel = {
> +   .reg_off = HHI_VID_PLL_CLK_DIV,
> +   .shift   = 16,
> +   .width   = 2,
> +   },
> +   },
> +   .hw.init = &(struct clk_init_data) {
> +   .name = "vid_pll_div",
> +   .ops = _vid_pll_div_ro_ops,
> +   .parent_names = (const char *[]){ "hdmi_pll" },
> +   .num_parents = 1,
> +   .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
> +   },
> +};
> +
> +const char *gxbb_vid_pll_parent_names[] = { "vid_pll_div", "hdmi_pll" };
> +
> +static struct clk_regmap gxbb_vid_pll_sel = {
> +   .data = &(struct clk_regmap_mux_data){
> +   .offset = HHI_VID_PLL_CLK_DIV,
> +   .mask = 0x1,
> +   .shift = 18,
> +   },
> +   .hw.init = &(struct clk_init_data){
> +   .name = "vid_pll_sel",
> +   .ops = _regmap_mux_ops,
> +   /*
> +* bit 18 selects from 2 possible parents:
> +* vid_pll_div or hdmi_pll
> +*/
> +   .parent_names = gxbb_vid_pll_parent_names,
> +   .num_parents = ARRAY_SIZE(gxbb_vid_pll_parent_names),
you could get rid of the comment if you inline
gxbb_vid_pll_parent_names and set num_parents to 2 manually

> +   .flags = CLK_SET_RATE_NO_REPARENT | CLK_GET_RATE_NOCACHE,
> +   },
> +};
> +
> +static struct clk_regmap gxbb_vid_pll = {
> +   .data = &(struct clk_regmap_gate_data){
> +   .offset = HHI_VID_PLL_CLK_DIV,
> +   .bit_idx = 19,
> +   },
> +   .hw.init = &(struct clk_init_data) {
> +   .name = "vid_pll",
> +   .ops = _regmap_gate_ops,
> +   .parent_names = (const char *[]){ "vid_pll_sel" },
> +   .num_parents = 1,
> +   .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> +   },
> +};
> +
> +const char *gxbb_vclk_parent_names[] = {
> +   "vid_pll", "fclk_div4", "fclk_div3", "fclk_div5", "vid_pll",
> +   "fclk_div7", "mpll1",
> +};
> +
> +static struct clk_regmap gxbb_vclk_sel = {
> +   .data = &(struct clk_regmap_mux_data){
> +   .offset = HHI_VID_CLK_CNTL,
> +   .mask = 0x7,
> +   .shift = 16,
> +   },
> +   .hw.init = &(struct clk_init_data){
> +   .name = "vclk_sel",
> +   .ops = _regmap_mux_ops,
> +   /*
> +* bits 16:18 selects from 8 possible parents:
> +* vid_pll, fclk_div4, fclk_div3, fclk_div5,
> +* vid_pll, fclk_div7, mp1
> +*/
does it select from 7 or 8 parents? you are listing only 7
the same applies for the usages below

> +   .parent_names = gxbb_vclk_parent_names,
> +   .num_parents = ARRAY_SIZE(gxbb_vclk_parent_names),
> +   .flags = CLK_SET_RATE_NO_REPARENT | CLK_GET_RATE_NOCACHE,
> +   },
> +};
> +
> +static struct clk_regmap gxbb_vclk2_sel = {
> +   .data = &(struct clk_regmap_mux_data){
> +   .offset = HHI_VIID_CLK_CNTL,
> +   .mask = 0x7,
> +   .shift = 16,
> +   },
> +   .hw.init = &(struct clk_init_data){
> +   .name = "vclk2_sel",
> +   .ops = _regmap_mux_ops,
> +   /*
> +* bits 16:18 selects from 8 possible parents:
> +* vid_pll, fclk_div4, fclk_div3, fclk_div5,
> +* vid_pll, fclk_div7, mp1
> +*/
> +   

Disabling CPU vulnerabilities workarounds (second try)

[Artem S. Tashkinov]

Hello,

I'm resending my last email since the first one didn't draw enough 
attention despite the gravity of the situation and the issue has been 
exacerbated by the recent kernel 4.20 changes which incur even a larger 
performance loss - up to 50% according to the most recent Phoronix testing:


https://www.phoronix.com/scan.php?page=article=linux-420-stibp

It looks like only pure compute loads are not affected by the new code 
which renders hyper-threading in Intel CPUs almost useless.




The original email follows:

***

As time goes by more and more fixes of Intel/AMD/ARM CPUs 
vulnerabilities are added to the Linux kernel without a simple way to 
disable them *all*.


Disabling is a good option for strictly confined environments where no 
third-party untrusted code is ever to be run, e.g. a rendering farm, a 
supercomputer, or even a home server which runs Samba/SSH server and 
nothing else.


I wonder if someone could write a patch which implements the following 
two options for the kernel:


* A boot option which allows to disable *most* 
protections/workarounds/fixes (as far as I understand some of them can't 
be reverted since they are compiled-in or use certain GCC workarounds), 
e.g. let's call it "insecure" or "insecurecpumode"


* A compile-time config option which disables the said fixes 
_permanently_ without a way to turn them back on.


Right now linux/Documentation/admin-guide/kernel-parameters.txt is a 
mess of various things which take ages to sift through and there's zero 
understanding whether you've found everything and correctly disabled it.


***


Addendum:

I can imagine that writing such a patch is not trivial and no one is 
eager to do that. In this case people would love to see an extra file in 
the kernel documentation, e.g. CPU-vulnerabilities.txt which lists all 
the existing protections in the kernel and the boot options to disable them.


The Internet is already rife with questions how to disable the said 
protections and the results are quite different.


In short, it would be great to have some organization in regard to the 
issue.



Regards,
Artem

Disabling CPU vulnerabilities workarounds (second try)

[Artem S. Tashkinov]

Hello,

I'm resending my last email since the first one didn't draw enough 
attention despite the gravity of the situation and the issue has been 
exacerbated by the recent kernel 4.20 changes which incur even a larger 
performance loss - up to 50% according to the most recent Phoronix testing:


https://www.phoronix.com/scan.php?page=article=linux-420-stibp

It looks like only pure compute loads are not affected by the new code 
which renders hyper-threading in Intel CPUs almost useless.




The original email follows:

***

As time goes by more and more fixes of Intel/AMD/ARM CPUs 
vulnerabilities are added to the Linux kernel without a simple way to 
disable them *all*.


Disabling is a good option for strictly confined environments where no 
third-party untrusted code is ever to be run, e.g. a rendering farm, a 
supercomputer, or even a home server which runs Samba/SSH server and 
nothing else.


I wonder if someone could write a patch which implements the following 
two options for the kernel:


* A boot option which allows to disable *most* 
protections/workarounds/fixes (as far as I understand some of them can't 
be reverted since they are compiled-in or use certain GCC workarounds), 
e.g. let's call it "insecure" or "insecurecpumode"


* A compile-time config option which disables the said fixes 
_permanently_ without a way to turn them back on.


Right now linux/Documentation/admin-guide/kernel-parameters.txt is a 
mess of various things which take ages to sift through and there's zero 
understanding whether you've found everything and correctly disabled it.


***


Addendum:

I can imagine that writing such a patch is not trivial and no one is 
eager to do that. In this case people would love to see an extra file in 
the kernel documentation, e.g. CPU-vulnerabilities.txt which lists all 
the existing protections in the kernel and the boot options to disable them.


The Internet is already rife with questions how to disable the said 
protections and the results are quite different.


In short, it would be great to have some organization in regard to the 
issue.



Regards,
Artem

Re: [PATCH v2 2/3] soc: amlogic: Add Meson GX Clock Measure driver

[Martin Blumenstingl]

Hi Neil,

On Wed, Nov 14, 2018 at 2:18 PM Neil Armstrong  wrote:
>
> The Amlogic Meson GX SoCs embeds a clock measurer IP to measure the internal
> clock paths frequencies.
I would remove "GX" from that sentence

> The precision is determined by stepping into the divider until the counter
> overflows.
> The debugfs slows a pretty summary and each clock can be measured
> individually aswell.
>
> The clock table covers all GX SoCs (GXBB, GXL & GXM) even if some clocks
> are only present on GXBB or GXM (i.e. wave420l) but the counter returns
> 0 when the clocks are invalid.
>
> Signed-off-by: Neil Armstrong 
looks good apart from a small comments (and a 32-bit SoC fix) below as
this driver is not GX specific

> ---
>  drivers/soc/amlogic/Kconfig|   8 +
>  drivers/soc/amlogic/Makefile   |   1 +
>  drivers/soc/amlogic/meson-gx-clk-measure.c | 293 +
>  3 files changed, 302 insertions(+)
>  create mode 100644 drivers/soc/amlogic/meson-gx-clk-measure.c
>
> diff --git a/drivers/soc/amlogic/Kconfig b/drivers/soc/amlogic/Kconfig
> index 2f282b472912..155868830773 100644
> --- a/drivers/soc/amlogic/Kconfig
> +++ b/drivers/soc/amlogic/Kconfig
> @@ -7,6 +7,14 @@ config MESON_CANVAS
> help
>   Say yes to support the canvas IP for Amlogic SoCs.
>
> +config MESON_GX_CLK_MEASURE
> +   bool "Amlogic Meson GX SoC Clock Measure driver"
please drop the GX from the name, I verified that it also works on
Meson8 and Meson8b with very little changes

> +   depends on ARCH_MESON || COMPILE_TEST
> +   default ARCH_MESON
> +   help
> + Say yes to support of Measuring a set of internal SoC clocks
> + from the debugfs interface.
> +
>  config MESON_GX_SOCINFO
> bool "Amlogic Meson GX SoC Information driver"
> depends on ARCH_MESON || COMPILE_TEST
> diff --git a/drivers/soc/amlogic/Makefile b/drivers/soc/amlogic/Makefile
> index 0ab16d35ac36..2cc0f9c2f715 100644
> --- a/drivers/soc/amlogic/Makefile
> +++ b/drivers/soc/amlogic/Makefile
> @@ -1,4 +1,5 @@
>  obj-$(CONFIG_MESON_CANVAS) += meson-canvas.o
> +obj-$(CONFIG_MESON_GX_CLK_MEASURE) += meson-gx-clk-measure.o
>  obj-$(CONFIG_MESON_GX_SOCINFO) += meson-gx-socinfo.o
>  obj-$(CONFIG_MESON_GX_PM_DOMAINS) += meson-gx-pwrc-vpu.o
>  obj-$(CONFIG_MESON_MX_SOCINFO) += meson-mx-socinfo.o
> diff --git a/drivers/soc/amlogic/meson-gx-clk-measure.c 
> b/drivers/soc/amlogic/meson-gx-clk-measure.c
> new file mode 100644
> index ..da1a0001cef9
> --- /dev/null
> +++ b/drivers/soc/amlogic/meson-gx-clk-measure.c
> @@ -0,0 +1,293 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2017 BayLibre, SAS
nitpick; 2017-2018

> + * Author: Neil Armstrong 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define MSR_CLK_DUTY   0x0
> +#define MSR_CLK_REG0   0x4
> +#define MSR_CLK_REG1   0x8
> +#define MSR_CLK_REG2   0xc
> +
> +#define MSR_CLK_DIVGENMASK(15, 0)
I would call this MSR_DURATION (see my comment on the divider below)

> +#define MSR_ENABLE BIT(16)
> +#define MSR_CONT   BIT(17) /* continuous measurement */
> +#define MSR_INTR   BIT(18) /* interrupts */
> +#define MSR_RUNBIT(19)
> +#define MSR_CLK_SRCGENMASK(26, 20)
> +#define MSR_BUSY   BIT(31)
> +
> +#define MSR_VAL_MASK   GENMASK(15, 0)
> +
> +#define DIV_MIN32
> +#define DIV_STEP   32
> +#define DIV_MAX640
> +
> +#define CLK_MSR_MAX128
> +
> +struct meson_gx_msr_id {
> +   struct meson_gx_msr *priv;
> +   unsigned int id;
> +   const char *name;
> +};
> +
> +struct meson_gx_msr {
> +   struct regmap *regmap;
> +   struct meson_gx_msr_id msr_table[CLK_MSR_MAX];
> +};
can you drop the "_gx" from these two structs and all functions below please?

> +
> +#define CLK_MSR_ID(__id, __name) \
> +   [__id] = {.id = __id, .name = __name,}
> +
> +static struct meson_gx_msr_id clk_msr_gx[CLK_MSR_MAX] = {
> +   CLK_MSR_ID(0, "ring_osc_out_ee_0"),
> +   CLK_MSR_ID(1, "ring_osc_out_ee_1"),
> +   CLK_MSR_ID(2, "ring_osc_out_ee_2"),
> +   CLK_MSR_ID(3, "a53_ring_osc"),
> +   CLK_MSR_ID(4, "gp0_pll"),
> +   CLK_MSR_ID(6, "enci"),
> +   CLK_MSR_ID(7, "clk81"),
> +   CLK_MSR_ID(8, "encp"),
> +   CLK_MSR_ID(9, "encl"),
> +   CLK_MSR_ID(10, "vdac"),
> +   CLK_MSR_ID(11, "rgmii_tx"),
> +   CLK_MSR_ID(12, "pdm"),
> +   CLK_MSR_ID(13, "amclk"),
> +   CLK_MSR_ID(14, "fec_0"),
> +   CLK_MSR_ID(15, "fec_1"),
> +   CLK_MSR_ID(16, "fec_2"),
> +   CLK_MSR_ID(17, "sys_pll_div16"),
> +   CLK_MSR_ID(18, "sys_cpu_div16"),
> +   CLK_MSR_ID(19, "hdmitx_sys"),
> +   CLK_MSR_ID(20, "rtc_osc_out"),
> +   CLK_MSR_ID(21, "i2s_in_src0"),
> +   CLK_MSR_ID(22, "eth_phy_ref"),
> +   

Re: [PATCH v2 2/3] soc: amlogic: Add Meson GX Clock Measure driver

[Martin Blumenstingl]

Hi Neil,

On Wed, Nov 14, 2018 at 2:18 PM Neil Armstrong  wrote:
>
> The Amlogic Meson GX SoCs embeds a clock measurer IP to measure the internal
> clock paths frequencies.
I would remove "GX" from that sentence

> The precision is determined by stepping into the divider until the counter
> overflows.
> The debugfs slows a pretty summary and each clock can be measured
> individually aswell.
>
> The clock table covers all GX SoCs (GXBB, GXL & GXM) even if some clocks
> are only present on GXBB or GXM (i.e. wave420l) but the counter returns
> 0 when the clocks are invalid.
>
> Signed-off-by: Neil Armstrong 
looks good apart from a small comments (and a 32-bit SoC fix) below as
this driver is not GX specific

> ---
>  drivers/soc/amlogic/Kconfig|   8 +
>  drivers/soc/amlogic/Makefile   |   1 +
>  drivers/soc/amlogic/meson-gx-clk-measure.c | 293 +
>  3 files changed, 302 insertions(+)
>  create mode 100644 drivers/soc/amlogic/meson-gx-clk-measure.c
>
> diff --git a/drivers/soc/amlogic/Kconfig b/drivers/soc/amlogic/Kconfig
> index 2f282b472912..155868830773 100644
> --- a/drivers/soc/amlogic/Kconfig
> +++ b/drivers/soc/amlogic/Kconfig
> @@ -7,6 +7,14 @@ config MESON_CANVAS
> help
>   Say yes to support the canvas IP for Amlogic SoCs.
>
> +config MESON_GX_CLK_MEASURE
> +   bool "Amlogic Meson GX SoC Clock Measure driver"
please drop the GX from the name, I verified that it also works on
Meson8 and Meson8b with very little changes

> +   depends on ARCH_MESON || COMPILE_TEST
> +   default ARCH_MESON
> +   help
> + Say yes to support of Measuring a set of internal SoC clocks
> + from the debugfs interface.
> +
>  config MESON_GX_SOCINFO
> bool "Amlogic Meson GX SoC Information driver"
> depends on ARCH_MESON || COMPILE_TEST
> diff --git a/drivers/soc/amlogic/Makefile b/drivers/soc/amlogic/Makefile
> index 0ab16d35ac36..2cc0f9c2f715 100644
> --- a/drivers/soc/amlogic/Makefile
> +++ b/drivers/soc/amlogic/Makefile
> @@ -1,4 +1,5 @@
>  obj-$(CONFIG_MESON_CANVAS) += meson-canvas.o
> +obj-$(CONFIG_MESON_GX_CLK_MEASURE) += meson-gx-clk-measure.o
>  obj-$(CONFIG_MESON_GX_SOCINFO) += meson-gx-socinfo.o
>  obj-$(CONFIG_MESON_GX_PM_DOMAINS) += meson-gx-pwrc-vpu.o
>  obj-$(CONFIG_MESON_MX_SOCINFO) += meson-mx-socinfo.o
> diff --git a/drivers/soc/amlogic/meson-gx-clk-measure.c 
> b/drivers/soc/amlogic/meson-gx-clk-measure.c
> new file mode 100644
> index ..da1a0001cef9
> --- /dev/null
> +++ b/drivers/soc/amlogic/meson-gx-clk-measure.c
> @@ -0,0 +1,293 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2017 BayLibre, SAS
nitpick; 2017-2018

> + * Author: Neil Armstrong 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define MSR_CLK_DUTY   0x0
> +#define MSR_CLK_REG0   0x4
> +#define MSR_CLK_REG1   0x8
> +#define MSR_CLK_REG2   0xc
> +
> +#define MSR_CLK_DIVGENMASK(15, 0)
I would call this MSR_DURATION (see my comment on the divider below)

> +#define MSR_ENABLE BIT(16)
> +#define MSR_CONT   BIT(17) /* continuous measurement */
> +#define MSR_INTR   BIT(18) /* interrupts */
> +#define MSR_RUNBIT(19)
> +#define MSR_CLK_SRCGENMASK(26, 20)
> +#define MSR_BUSY   BIT(31)
> +
> +#define MSR_VAL_MASK   GENMASK(15, 0)
> +
> +#define DIV_MIN32
> +#define DIV_STEP   32
> +#define DIV_MAX640
> +
> +#define CLK_MSR_MAX128
> +
> +struct meson_gx_msr_id {
> +   struct meson_gx_msr *priv;
> +   unsigned int id;
> +   const char *name;
> +};
> +
> +struct meson_gx_msr {
> +   struct regmap *regmap;
> +   struct meson_gx_msr_id msr_table[CLK_MSR_MAX];
> +};
can you drop the "_gx" from these two structs and all functions below please?

> +
> +#define CLK_MSR_ID(__id, __name) \
> +   [__id] = {.id = __id, .name = __name,}
> +
> +static struct meson_gx_msr_id clk_msr_gx[CLK_MSR_MAX] = {
> +   CLK_MSR_ID(0, "ring_osc_out_ee_0"),
> +   CLK_MSR_ID(1, "ring_osc_out_ee_1"),
> +   CLK_MSR_ID(2, "ring_osc_out_ee_2"),
> +   CLK_MSR_ID(3, "a53_ring_osc"),
> +   CLK_MSR_ID(4, "gp0_pll"),
> +   CLK_MSR_ID(6, "enci"),
> +   CLK_MSR_ID(7, "clk81"),
> +   CLK_MSR_ID(8, "encp"),
> +   CLK_MSR_ID(9, "encl"),
> +   CLK_MSR_ID(10, "vdac"),
> +   CLK_MSR_ID(11, "rgmii_tx"),
> +   CLK_MSR_ID(12, "pdm"),
> +   CLK_MSR_ID(13, "amclk"),
> +   CLK_MSR_ID(14, "fec_0"),
> +   CLK_MSR_ID(15, "fec_1"),
> +   CLK_MSR_ID(16, "fec_2"),
> +   CLK_MSR_ID(17, "sys_pll_div16"),
> +   CLK_MSR_ID(18, "sys_cpu_div16"),
> +   CLK_MSR_ID(19, "hdmitx_sys"),
> +   CLK_MSR_ID(20, "rtc_osc_out"),
> +   CLK_MSR_ID(21, "i2s_in_src0"),
> +   CLK_MSR_ID(22, "eth_phy_ref"),
> +   

Re: [PATCH v2 1/3] dt-bindings: amlogic: Add Internal Clock Measurer bindings

[Martin Blumenstingl]

Hi Neil,

On Wed, Nov 14, 2018 at 2:16 PM Neil Armstrong  wrote:
>
> The Amlogic Meson GX SoCs embeds a clock measurer IP to measure the internal
> clock paths frequencies.
I would skip the "GX" in that sentence: there's a similar (identical?)
clock measurer in Meson6, Meson8, Meson8b and Meson8m2 as well

>
> Signed-off-by: Neil Armstrong 
with the comment above and below:
Acked-by: Martin Blumenstingl 

> ---
>  .../bindings/soc/amlogic/clk-measure.txt  | 15 +++
>  1 file changed, 15 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/soc/amlogic/clk-measure.txt
>
> diff --git a/Documentation/devicetree/bindings/soc/amlogic/clk-measure.txt 
> b/Documentation/devicetree/bindings/soc/amlogic/clk-measure.txt
> new file mode 100644
> index ..ba9183a42032
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/amlogic/clk-measure.txt
> @@ -0,0 +1,15 @@
> +Amlogic Internal Clock Measurer
> +===
> +
> +The Amlogic SoCs contains an IP to measure the internal clocks.
> +The precision is multiple of MHz, useful to debug the clock states.
> +
> +Required properties:
> +- compatible: Shall contain "amlogic,meson-gx-clk-measure"
can you please add "amlogic,meson8-clk-measure" and
"amlogic,meson8b-clk-measure" as well?
(I will provide a patch on top of yours to add Meson8 and Meson8b support)


Regards
Martin

Re: [PATCH v2 1/3] dt-bindings: amlogic: Add Internal Clock Measurer bindings

[Martin Blumenstingl]

Hi Neil,

On Wed, Nov 14, 2018 at 2:16 PM Neil Armstrong  wrote:
>
> The Amlogic Meson GX SoCs embeds a clock measurer IP to measure the internal
> clock paths frequencies.
I would skip the "GX" in that sentence: there's a similar (identical?)
clock measurer in Meson6, Meson8, Meson8b and Meson8m2 as well

>
> Signed-off-by: Neil Armstrong 
with the comment above and below:
Acked-by: Martin Blumenstingl 

> ---
>  .../bindings/soc/amlogic/clk-measure.txt  | 15 +++
>  1 file changed, 15 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/soc/amlogic/clk-measure.txt
>
> diff --git a/Documentation/devicetree/bindings/soc/amlogic/clk-measure.txt 
> b/Documentation/devicetree/bindings/soc/amlogic/clk-measure.txt
> new file mode 100644
> index ..ba9183a42032
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/amlogic/clk-measure.txt
> @@ -0,0 +1,15 @@
> +Amlogic Internal Clock Measurer
> +===
> +
> +The Amlogic SoCs contains an IP to measure the internal clocks.
> +The precision is multiple of MHz, useful to debug the clock states.
> +
> +Required properties:
> +- compatible: Shall contain "amlogic,meson-gx-clk-measure"
can you please add "amlogic,meson8-clk-measure" and
"amlogic,meson8b-clk-measure" as well?
(I will provide a patch on top of yours to add Meson8 and Meson8b support)


Regards
Martin

Re: [PATCH] Allow hwrng to initialize crng.

[Michael Niewöhner]

Hi Louis,

On Wed, 2018-09-26 at 11:24 +0800, Louis Collard wrote:
> Some systems, for example embedded systems, do not generate
> enough entropy on boot through interrupts, and boot may be blocked for
> several minutes waiting for a call to getrandom to complete.
> 
> Currently, random data is read from a hwrng when it is registered,
> and is loaded into primary_crng. This data is treated in the same
> way as data that is device-specific but otherwise unchanging, and
> so primary_crng cannot become initialized with the data from the
> hwrng.
> 
> This change causes the data initially read from the hwrng to be
> treated the same as subsequent data that is read from the hwrng if
> it's quality score is non-zero.
> 
> The implications of this are:
> 
> The data read from hwrng can cause primary_crng to become
> initialized, therefore avoiding problems of getrandom blocking
> on boot.
> 
> Calls to getrandom (with GRND_RANDOM) may be using entropy
> exclusively (or in practise, almost exclusively) from the hwrng.
> 
> Regarding the latter point; this behavior is the same as if a
> user specified a quality score of 1 (bit of entropy per 1024 bits)
> so hopefully this is not too scary a change to make.
> 
> This change is the result of the discussion here:
> https://patchwork.kernel.org/patch/10453893/
> 
> Signed-off-by: Louis Collard 
> Acked-by: Jarkko Sakkinen 
> ---
>  drivers/char/hw_random/core.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index aaf9e5afaad4..47f358aa0c3d 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #define RNG_MODULE_NAME  "hw_random"
>  
> @@ -64,13 +65,17 @@ static size_t rng_buffer_size(void)
>  static void add_early_randomness(struct hwrng *rng)
>  {
>   int bytes_read;
> - size_t size = min_t(size_t, 16, rng_buffer_size());
> + /* Read enough to initialize crng. */
> + size_t size = 2*CHACHA20_KEY_SIZE;
>  
>   mutex_lock(_mutex);
>   bytes_read = rng_get_data(rng, rng_buffer, size, 1);
>   mutex_unlock(_mutex);
>   if (bytes_read > 0)
> - add_device_randomness(rng_buffer, bytes_read);
> + /* Allow crng to become initialized, but do not add
> +  * entropy to the pool.
> +  */
> + add_hwgenerator_randomness(rng_buffer, bytes_read, 0);
>  }
>  
>  static inline void cleanup_rng(struct kref *kref)

I found your patch by chance, searching for a solution for crng init delay on my
headless machine. Unfortunately it hardly makes any difference for me. With the
patch the system hangs for about 80s instead of 120s until the "crng init done"
message.In contrast, doing a `cat /dev/hwrng >/dev/random` or running rngd
initializes the crng instantly.

Isn't that delay the problem this patch tries to fix? Any idea what is wrong
here?

Thanks!

Best regards
Michael

Re: [PATCH] Allow hwrng to initialize crng.

[Michael Niewöhner]

Hi Louis,

On Wed, 2018-09-26 at 11:24 +0800, Louis Collard wrote:
> Some systems, for example embedded systems, do not generate
> enough entropy on boot through interrupts, and boot may be blocked for
> several minutes waiting for a call to getrandom to complete.
> 
> Currently, random data is read from a hwrng when it is registered,
> and is loaded into primary_crng. This data is treated in the same
> way as data that is device-specific but otherwise unchanging, and
> so primary_crng cannot become initialized with the data from the
> hwrng.
> 
> This change causes the data initially read from the hwrng to be
> treated the same as subsequent data that is read from the hwrng if
> it's quality score is non-zero.
> 
> The implications of this are:
> 
> The data read from hwrng can cause primary_crng to become
> initialized, therefore avoiding problems of getrandom blocking
> on boot.
> 
> Calls to getrandom (with GRND_RANDOM) may be using entropy
> exclusively (or in practise, almost exclusively) from the hwrng.
> 
> Regarding the latter point; this behavior is the same as if a
> user specified a quality score of 1 (bit of entropy per 1024 bits)
> so hopefully this is not too scary a change to make.
> 
> This change is the result of the discussion here:
> https://patchwork.kernel.org/patch/10453893/
> 
> Signed-off-by: Louis Collard 
> Acked-by: Jarkko Sakkinen 
> ---
>  drivers/char/hw_random/core.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index aaf9e5afaad4..47f358aa0c3d 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #define RNG_MODULE_NAME  "hw_random"
>  
> @@ -64,13 +65,17 @@ static size_t rng_buffer_size(void)
>  static void add_early_randomness(struct hwrng *rng)
>  {
>   int bytes_read;
> - size_t size = min_t(size_t, 16, rng_buffer_size());
> + /* Read enough to initialize crng. */
> + size_t size = 2*CHACHA20_KEY_SIZE;
>  
>   mutex_lock(_mutex);
>   bytes_read = rng_get_data(rng, rng_buffer, size, 1);
>   mutex_unlock(_mutex);
>   if (bytes_read > 0)
> - add_device_randomness(rng_buffer, bytes_read);
> + /* Allow crng to become initialized, but do not add
> +  * entropy to the pool.
> +  */
> + add_hwgenerator_randomness(rng_buffer, bytes_read, 0);
>  }
>  
>  static inline void cleanup_rng(struct kref *kref)

I found your patch by chance, searching for a solution for crng init delay on my
headless machine. Unfortunately it hardly makes any difference for me. With the
patch the system hangs for about 80s instead of 120s until the "crng init done"
message.In contrast, doing a `cat /dev/hwrng >/dev/random` or running rngd
initializes the crng instantly.

Isn't that delay the problem this patch tries to fix? Any idea what is wrong
here?

Thanks!

Best regards
Michael

U.S. Customs and Border??

[Ronald Vitiello]

--
U.S. Customs and Border
Protection Acting Deputy
Commissioner

I am writing. in regards of your Package in our custody. with Mr. Jerry 
A. Benson, A Diplomats from Global Agent Service was stopped at the 
border in possession of the above mention metal box weight of about 25kg 
(Internal dimension: W61 x H156 x D73 (cm). Effective capacity: 110 
LBS.). Approximately, destine to your name and home address


He could not produce all necessary clearance papers which made us to put 
your Package on hold until we receive the Customs Registration number 
from you to prove your ownership. You are hereby giving a grace of 
96hours to provide the Customs Registration number, otherwise your 
Package will be confiscated and send back to the Treasury Office


The Registration number is very necessary to enable us verify that you 
are the real beneficiary of the metal box, after which your Package will 
be release to you* to avoid any necessary delay, we will advise that you 
re-confirm your full name, postal address and contact information for 
faster and smooth communications.


NOTE: If You Receive This Message In Your Junk Or Spam It's Due To Your 
Internet Provider. As soon as you receive this email and send me all the 
requested information to my direct email address:  
ronaldvitie...@aol.com


U.S. Customs and Border
Protection Acting Deputy
Commissioner
Ronald.D.Vitiello

U.S. Customs and Border??

[Ronald Vitiello]

--
U.S. Customs and Border
Protection Acting Deputy
Commissioner

I am writing. in regards of your Package in our custody. with Mr. Jerry 
A. Benson, A Diplomats from Global Agent Service was stopped at the 
border in possession of the above mention metal box weight of about 25kg 
(Internal dimension: W61 x H156 x D73 (cm). Effective capacity: 110 
LBS.). Approximately, destine to your name and home address


He could not produce all necessary clearance papers which made us to put 
your Package on hold until we receive the Customs Registration number 
from you to prove your ownership. You are hereby giving a grace of 
96hours to provide the Customs Registration number, otherwise your 
Package will be confiscated and send back to the Treasury Office


The Registration number is very necessary to enable us verify that you 
are the real beneficiary of the metal box, after which your Package will 
be release to you* to avoid any necessary delay, we will advise that you 
re-confirm your full name, postal address and contact information for 
faster and smooth communications.


NOTE: If You Receive This Message In Your Junk Or Spam It's Due To Your 
Internet Provider. As soon as you receive this email and send me all the 
requested information to my direct email address:  
ronaldvitie...@aol.com


U.S. Customs and Border
Protection Acting Deputy
Commissioner
Ronald.D.Vitiello

U.S. Customs and Border??

[Ronald Vitiello]

--
U.S. Customs and Border
Protection Acting Deputy
Commissioner

I am writing. in regards of your Package in our custody. with Mr. Jerry 
A. Benson, A Diplomats from Global Agent Service was stopped at the 
border in possession of the above mention metal box weight of about 25kg 
(Internal dimension: W61 x H156 x D73 (cm). Effective capacity: 110 
LBS.). Approximately, destine to your name and home address


He could not produce all necessary clearance papers which made us to put 
your Package on hold until we receive the Customs Registration number 
from you to prove your ownership. You are hereby giving a grace of 
96hours to provide the Customs Registration number, otherwise your 
Package will be confiscated and send back to the Treasury Office


The Registration number is very necessary to enable us verify that you 
are the real beneficiary of the metal box, after which your Package will 
be release to you* to avoid any necessary delay, we will advise that you 
re-confirm your full name, postal address and contact information for 
faster and smooth communications.


NOTE: If You Receive This Message In Your Junk Or Spam It's Due To Your 
Internet Provider. As soon as you receive this email and send me all the 
requested information to my direct email address:  
ronaldvitie...@aol.com


U.S. Customs and Border
Protection Acting Deputy
Commissioner
Ronald.D.Vitiello

U.S. Customs and Border??

[Ronald Vitiello]

--
U.S. Customs and Border
Protection Acting Deputy
Commissioner

I am writing. in regards of your Package in our custody. with Mr. Jerry 
A. Benson, A Diplomats from Global Agent Service was stopped at the 
border in possession of the above mention metal box weight of about 25kg 
(Internal dimension: W61 x H156 x D73 (cm). Effective capacity: 110 
LBS.). Approximately, destine to your name and home address


He could not produce all necessary clearance papers which made us to put 
your Package on hold until we receive the Customs Registration number 
from you to prove your ownership. You are hereby giving a grace of 
96hours to provide the Customs Registration number, otherwise your 
Package will be confiscated and send back to the Treasury Office


The Registration number is very necessary to enable us verify that you 
are the real beneficiary of the metal box, after which your Package will 
be release to you* to avoid any necessary delay, we will advise that you 
re-confirm your full name, postal address and contact information for 
faster and smooth communications.


NOTE: If You Receive This Message In Your Junk Or Spam It's Due To Your 
Internet Provider. As soon as you receive this email and send me all the 
requested information to my direct email address:  
ronaldvitie...@aol.com


U.S. Customs and Border
Protection Acting Deputy
Commissioner
Ronald.D.Vitiello