From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755391AbcE0PR1 (ORCPT ); Fri, 27 May 2016 11:17:27 -0400 Received: from mail-wm0-f54.google.com ([74.125.82.54]:36341 "EHLO mail-wm0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750850AbcE0PRZ (ORCPT ); Fri, 27 May 2016 11:17:25 -0400 Subject: Re: [RFC PATCH 0/2] scpi: Add SCPI framework to handle vendors variants To: Sudeep Holla References: <1464255491-18503-1-git-send-email-narmstrong@baylibre.com> <57472450.4000709@arm.com> <574802AF.2080909@baylibre.com> Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org From: Neil Armstrong Organization: Baylibre Message-ID: <5748651C.9060901@baylibre.com> Date: Fri, 27 May 2016 17:17:48 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <574802AF.2080909@baylibre.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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