mbox series

[GIT,PULL] firmware: arm_scpi: updates/cleanups for v4.15

Message ID 901d3720-a023-0af9-2b20-b7d7e8528571@arm.com
State New
Headers show
Series [GIT,PULL] firmware: arm_scpi: updates/cleanups for v4.15 | expand

Pull-request

git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git

Message

Sudeep Holla Oct. 9, 2017, 5:27 p.m. UTC
Hi ARM SoC Team,

Please pull !

Regards,
Sudeep

--

The following changes since commit 2bd6bf03f4c1c59381d62c61d03f6cc3fe71f66e:

  Linux 4.14-rc1 (2017-09-16 15:47:51 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git
tags/scpi-updates-4.15

for you to fetch changes up to 430594c1c7f5051f0d99ed9d08d086d20587cdd1:

  firmware: arm_scpi: silence sparse warnings (2017-10-09 10:20:02 +0100)

----------------------------------------------------------------
ARM SCPI updates/cleanups for v4.15

1. Fixes to get rid of sparse warnings
2. Use of FIELD_GET and GENMASK for better subfields handling
3. Make mbox_free_channels device-managed helping in removing
unnecessary code
4. Various other cleanups to simplify and improve code readability

----------------------------------------------------------------
Heiner Kallweit (9):
      firmware: arm_scpi: remove usage of drvdata and don't reset
scpi_info to null
      firmware: arm_scpi: remove two unneeded devm_kfree's in scpi_remove
      firmware: arm_scpi: pre-populate dvfs info in scpi_probe
      firmware: arm_scpi: make freeing mbox channels device-managed
      firmware: arm_scpi: remove scpi_remove
      firmware: arm_scpi: improve struct dvfs_info to make code better
readable
      firmware: arm_scpi: improve handling of protocol and firmware
version subfields
      firmware: arm_scpi: improve struct sensor_value
      firmware: arm_scpi: silence sparse warnings

Sudeep Holla (2):
      firmware: arm_scpi: drop unnecessary type cast to scpi_shared_mem
      firmware: arm_scpi: remove all single element structures

 drivers/firmware/arm_scpi.c | 216
++++++++++++++++++--------------------------
 1 file changed, 87 insertions(+), 129 deletions(-)

Comments

Arnd Bergmann Oct. 20, 2017, 8:33 p.m. UTC | #1
On Mon, Oct 9, 2017 at 7:27 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> The following changes since commit 2bd6bf03f4c1c59381d62c61d03f6cc3fe71f66e:
>
>   Linux 4.14-rc1 (2017-09-16 15:47:51 -0700)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git
> tags/scpi-updates-4.15
>
> for you to fetch changes up to 430594c1c7f5051f0d99ed9d08d086d20587cdd1:
>
>   firmware: arm_scpi: silence sparse warnings (2017-10-09 10:20:02 +0100)
>
> ----------------------------------------------------------------
> ARM SCPI updates/cleanups for v4.15
>
> 1. Fixes to get rid of sparse warnings
> 2. Use of FIELD_GET and GENMASK for better subfields handling
> 3. Make mbox_free_channels device-managed helping in removing
> unnecessary code
> 4. Various other cleanups to simplify and improve code readability

Pulled into next/drivers. I'm a little unsure about the first patch: the
resource is called "shmem", which suggests that you are dealing
with memory and should use "memremap()" instead of "ioremap()"
and readl/writel. Can you clarify what the mapping attributes are
supposed to be here? Thanks.

        Arnd
Sudeep Holla Oct. 23, 2017, 9 a.m. UTC | #2
On 20/10/17 21:33, Arnd Bergmann wrote:
> On Mon, Oct 9, 2017 at 7:27 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>> The following changes since commit 2bd6bf03f4c1c59381d62c61d03f6cc3fe71f66e:
>>
>>   Linux 4.14-rc1 (2017-09-16 15:47:51 -0700)
>>
>> are available in the git repository at:
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git
>> tags/scpi-updates-4.15
>>
>> for you to fetch changes up to 430594c1c7f5051f0d99ed9d08d086d20587cdd1:
>>
>>   firmware: arm_scpi: silence sparse warnings (2017-10-09 10:20:02 +0100)
>>
>> ----------------------------------------------------------------
>> ARM SCPI updates/cleanups for v4.15
>>
>> 1. Fixes to get rid of sparse warnings
>> 2. Use of FIELD_GET and GENMASK for better subfields handling
>> 3. Make mbox_free_channels device-managed helping in removing
>> unnecessary code
>> 4. Various other cleanups to simplify and improve code readability
> 
> Pulled into next/drivers. I'm a little unsure about the first patch: the
> resource is called "shmem", which suggests that you are dealing
> with memory and should use "memremap()" instead of "ioremap()"
> and readl/writel. Can you clarify what the mapping attributes are
> supposed to be here? Thanks.

This is the shared memory carved out of SRAM. Since it's shared with
remote processor it's prefer to have device ordering and uncached.
drivers/misc/sram.c does have ioremap, the shmem used here is reserved
region from SRAM. Let me know if this sounds fine.
Arnd Bergmann Oct. 30, 2017, 9:11 a.m. UTC | #3
On Mon, Oct 23, 2017 at 11:00 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 20/10/17 21:33, Arnd Bergmann wrote:
>> On Mon, Oct 9, 2017 at 7:27 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>
>>> The following changes since commit 2bd6bf03f4c1c59381d62c61d03f6cc3fe71f66e:
>>>
>>>   Linux 4.14-rc1 (2017-09-16 15:47:51 -0700)
>>>
>>> are available in the git repository at:
>>>
>>>   git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git
>>> tags/scpi-updates-4.15
>>>
>>> for you to fetch changes up to 430594c1c7f5051f0d99ed9d08d086d20587cdd1:
>>>
>>>   firmware: arm_scpi: silence sparse warnings (2017-10-09 10:20:02 +0100)
>>>
>>> ----------------------------------------------------------------
>>> ARM SCPI updates/cleanups for v4.15
>>>
>>> 1. Fixes to get rid of sparse warnings
>>> 2. Use of FIELD_GET and GENMASK for better subfields handling
>>> 3. Make mbox_free_channels device-managed helping in removing
>>> unnecessary code
>>> 4. Various other cleanups to simplify and improve code readability
>>
>> Pulled into next/drivers. I'm a little unsure about the first patch: the
>> resource is called "shmem", which suggests that you are dealing
>> with memory and should use "memremap()" instead of "ioremap()"
>> and readl/writel. Can you clarify what the mapping attributes are
>> supposed to be here? Thanks.
>
> This is the shared memory carved out of SRAM. Since it's shared with
> remote processor it's prefer to have device ordering and uncached.
> drivers/misc/sram.c does have ioremap, the shmem used here is reserved
> region from SRAM. Let me know if this sounds fine.

Ok, for SRAM, uncached access and __iomem pointers are correct.

Byte ordering tends to be a problem with this kind of data transfer,
as you usually have a combination of fixed-order fields that require
byte swaps and byte strings that must not be swapped. I'm fairly sure
you got these all right here, so there is no problem with your patches,
just something to remember for the future.

        Arnd
Sudeep Holla Oct. 30, 2017, 9:51 a.m. UTC | #4
On 30/10/17 09:11, Arnd Bergmann wrote:
> On Mon, Oct 23, 2017 at 11:00 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>>
>> On 20/10/17 21:33, Arnd Bergmann wrote:
>>> On Mon, Oct 9, 2017 at 7:27 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>>
>>>> The following changes since commit 2bd6bf03f4c1c59381d62c61d03f6cc3fe71f66e:
>>>>
>>>>   Linux 4.14-rc1 (2017-09-16 15:47:51 -0700)
>>>>
>>>> are available in the git repository at:
>>>>
>>>>   git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git
>>>> tags/scpi-updates-4.15
>>>>
>>>> for you to fetch changes up to 430594c1c7f5051f0d99ed9d08d086d20587cdd1:
>>>>
>>>>   firmware: arm_scpi: silence sparse warnings (2017-10-09 10:20:02 +0100)
>>>>
>>>> ----------------------------------------------------------------
>>>> ARM SCPI updates/cleanups for v4.15
>>>>
>>>> 1. Fixes to get rid of sparse warnings
>>>> 2. Use of FIELD_GET and GENMASK for better subfields handling
>>>> 3. Make mbox_free_channels device-managed helping in removing
>>>> unnecessary code
>>>> 4. Various other cleanups to simplify and improve code readability
>>>
>>> Pulled into next/drivers. I'm a little unsure about the first patch: the
>>> resource is called "shmem", which suggests that you are dealing
>>> with memory and should use "memremap()" instead of "ioremap()"
>>> and readl/writel. Can you clarify what the mapping attributes are
>>> supposed to be here? Thanks.
>>
>> This is the shared memory carved out of SRAM. Since it's shared with
>> remote processor it's prefer to have device ordering and uncached.
>> drivers/misc/sram.c does have ioremap, the shmem used here is reserved
>> region from SRAM. Let me know if this sounds fine.
> 
> Ok, for SRAM, uncached access and __iomem pointers are correct.
> 
Thanks for confirming.

> Byte ordering tends to be a problem with this kind of data transfer,
> as you usually have a combination of fixed-order fields that require
> byte swaps and byte strings that must not be swapped. I'm fairly sure
> you got these all right here, so there is no problem with your patches,
> just something to remember for the future.
> 

Indeed, the specification must explicitly specify that difference. In
case of SCPI and the new SCMI, it clearly specified.