From: narmstrong@baylibre.com (Neil Armstrong)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH 0/2] scpi: Add SCPI framework to handle vendors variants
Date: Fri, 27 May 2016 17:17:48 +0200 [thread overview]
Message-ID: <5748651C.9060901@baylibre.com> (raw)
In-Reply-To: <574802AF.2080909@baylibre.com>
On 05/27/2016 10:17 AM, Neil Armstrong wrote:
> Hi,
> On 05/26/2016 06:29 PM, Sudeep Holla wrote:
>> Hi Neil,
>>> To keep the spirit of the SCPI interface, add a thin "register" layer to get
>>> the ops from the parent node and switch the drivers using the ops to use
>>> the new of_scpi_ops_get() call.
>>>
>>
>> All I can see is that you share the code to register such drivers which
>> is hardly anything. It's hard to say all drivers use same protocol or
>> interface after this change. I may be missing to see something here so
>> it would be easy to appreciate/review this change with one user of this.
>
> I'm actually working on a "SCPI" implementation from Amlogic for their arm64 platform,
> and their implementation is slightly different but enough to need a separate driver.
>
> For instance, the differences are :
> - they do not implement virtual channels
> - they pass the command word by the mailbox interface
> - they use the "sender id" to group the command in a sort of "class"
> - the payload size is shifted by 20 instead of 16
> - the command word is not in the SRAM, so we cannot use a rx command queue, only a single command by channel at a single time
> - the command indexes are slightly shifted
> - in clk_set_value, "rate" is first instead of last entry
> - in sensor_value, they only use a 32bit entry
> - some commands are only accepted on the high priority channel, the others only on the low priority
> - MAX_DVFS_DOMAINS is 3 instead of 8
> - MAX_DVFS_OPPS is 16 instead of 8
>
> But, It's still a "SCPI" interface by design and usage because :
> - they implement 90% of the same commands, in the same way
> - the usage is exactly the same and architecture is similar
> - they use the same error code scheme
>
> Finally, it would be stupid to add an exact copy of the scpi-sensors and scpi-clocks !
> The scpi-cpufreq is another story since they only have a single A53 cluster, it's not adapted.
>
> This is why I wrote a very thin layer, and it can also clean up the arm_scpi and the scpi driver to use a cleaner registry interface.
> It would also be a good point to add a private_data to the ops and pass it to the scpi callbacks, to completely remove the global variables.
>
> I found an issue with scpi-cpufreq, since the device is created by the scpi-clocks probe, it does not have a proper node and my current of_scpi_ops_get won't work...
> The scpi-cpufreq should have a proper DT node and use the deffered probe to register after the scpi-clocks.
Actually I managed to make the Amlogic SCPI SCPI implementation work using a separate driver and the registry layer just fine.
The cpufreq works, like the scpi-dvfs-clocks and the scpi-hwmon drivers.
You can have a look at my github tree :
https://github.com/superna9999/linux/compare/28165ec7a99be98123aa89540bf2cfc24df19498...superna9999:amlogic/v4.7/scpi+fw-v0
The Amlogic SCPI implementation lies on top of this RFC patchset.
The current patchset need a fix in scpi-hwmon and for scpi-cpufreq but it works like a charm.
Neil
WARNING: multiple messages have this Message-ID (diff)
From: Neil Armstrong <narmstrong@baylibre.com>
To: Sudeep Holla <sudeep.holla@arm.com>
Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 0/2] scpi: Add SCPI framework to handle vendors variants
Date: Fri, 27 May 2016 17:17:48 +0200 [thread overview]
Message-ID: <5748651C.9060901@baylibre.com> (raw)
In-Reply-To: <574802AF.2080909@baylibre.com>
On 05/27/2016 10:17 AM, Neil Armstrong wrote:
> Hi,
> On 05/26/2016 06:29 PM, Sudeep Holla wrote:
>> Hi Neil,
>>> To keep the spirit of the SCPI interface, add a thin "register" layer to get
>>> the ops from the parent node and switch the drivers using the ops to use
>>> the new of_scpi_ops_get() call.
>>>
>>
>> All I can see is that you share the code to register such drivers which
>> is hardly anything. It's hard to say all drivers use same protocol or
>> interface after this change. I may be missing to see something here so
>> it would be easy to appreciate/review this change with one user of this.
>
> I'm actually working on a "SCPI" implementation from Amlogic for their arm64 platform,
> and their implementation is slightly different but enough to need a separate driver.
>
> For instance, the differences are :
> - they do not implement virtual channels
> - they pass the command word by the mailbox interface
> - they use the "sender id" to group the command in a sort of "class"
> - the payload size is shifted by 20 instead of 16
> - the command word is not in the SRAM, so we cannot use a rx command queue, only a single command by channel at a single time
> - the command indexes are slightly shifted
> - in clk_set_value, "rate" is first instead of last entry
> - in sensor_value, they only use a 32bit entry
> - some commands are only accepted on the high priority channel, the others only on the low priority
> - MAX_DVFS_DOMAINS is 3 instead of 8
> - MAX_DVFS_OPPS is 16 instead of 8
>
> But, It's still a "SCPI" interface by design and usage because :
> - they implement 90% of the same commands, in the same way
> - the usage is exactly the same and architecture is similar
> - they use the same error code scheme
>
> Finally, it would be stupid to add an exact copy of the scpi-sensors and scpi-clocks !
> The scpi-cpufreq is another story since they only have a single A53 cluster, it's not adapted.
>
> This is why I wrote a very thin layer, and it can also clean up the arm_scpi and the scpi driver to use a cleaner registry interface.
> It would also be a good point to add a private_data to the ops and pass it to the scpi callbacks, to completely remove the global variables.
>
> I found an issue with scpi-cpufreq, since the device is created by the scpi-clocks probe, it does not have a proper node and my current of_scpi_ops_get won't work...
> The scpi-cpufreq should have a proper DT node and use the deffered probe to register after the scpi-clocks.
Actually I managed to make the Amlogic SCPI SCPI implementation work using a separate driver and the registry layer just fine.
The cpufreq works, like the scpi-dvfs-clocks and the scpi-hwmon drivers.
You can have a look at my github tree :
https://github.com/superna9999/linux/compare/28165ec7a99be98123aa89540bf2cfc24df19498...superna9999:amlogic/v4.7/scpi+fw-v0
The Amlogic SCPI implementation lies on top of this RFC patchset.
The current patchset need a fix in scpi-hwmon and for scpi-cpufreq but it works like a charm.
Neil
next prev parent reply other threads:[~2016-05-27 15:17 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-26 9:38 [RFC PATCH 0/2] scpi: Add SCPI framework to handle vendors variants Neil Armstrong
2016-05-26 9:38 ` Neil Armstrong
2016-05-26 9:38 ` [RFC PATCH 1/2] firmware: Add a SCPI framework to handle multiple vendors implementation Neil Armstrong
2016-05-26 9:38 ` Neil Armstrong
2016-05-26 9:38 ` [RFC PATCH 2/2] firmware: scpi: Switch scpi drivers to use new Framework calls Neil Armstrong
2016-05-26 9:38 ` Neil Armstrong
2016-05-26 16:29 ` [RFC PATCH 0/2] scpi: Add SCPI framework to handle vendors variants Sudeep Holla
2016-05-26 16:29 ` Sudeep Holla
2016-05-27 8:17 ` Neil Armstrong
2016-05-27 8:17 ` Neil Armstrong
2016-05-27 15:17 ` Neil Armstrong [this message]
2016-05-27 15:17 ` Neil Armstrong
2016-05-30 8:30 ` Neil Armstrong
2016-05-30 8:30 ` Neil Armstrong
2016-06-01 10:10 ` Sudeep Holla
2016-06-01 10:10 ` Sudeep Holla
2016-06-01 16:30 ` Kevin Hilman
2016-06-01 16:30 ` Kevin Hilman
2016-06-01 16:34 ` Sudeep Holla
2016-06-01 16:34 ` Sudeep Holla
2016-06-01 18:48 ` Heiko Stübner
2016-06-01 18:48 ` Heiko Stübner
2016-06-06 17:10 ` Sudeep Holla
2016-06-06 17:10 ` Sudeep Holla
2016-06-20 10:25 ` Neil Armstrong
2016-06-20 10:25 ` Neil Armstrong
2016-06-20 15:01 ` Sudeep Holla
2016-06-20 15:01 ` Sudeep Holla
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5748651C.9060901@baylibre.com \
--to=narmstrong@baylibre.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.