Re: [PATCH 1/2] soc: imx-sc: add i.MX system controller soc driver support

[Leonard Crestez]

On 4/22/2019 9:46 AM, Anson Huang wrote:
>> -----Original Message-----
>> From: Anson Huang
>>> From: Shawn Guo [mailto:shawnguo@xxxxxxxxxx]
>>> On Sun, Apr 21, 2019 at 03:40:00PM +0800, Shawn Guo wrote:
>>>> On Thu, Apr 11, 2019 at 06:49:12AM +0000, Anson Huang wrote:
>>>>> i.MX8QXP is an ARMv8 SoC which has a Cortex-M4 system controller
>>>>> inside, the system controller is in charge of controlling power,
>>>>> clock and fuse etc..
>>>>>
>>>>> This patch adds i.MX system controller soc driver support, Linux
>>>>> kernel has to communicate with system controller via MU (message
>>>>> unit) IPC to get soc revision, uid etc..
>>>>>
>>>>> With this patch, soc info can be read from sysfs:
>>>>>
>>>>>   drivers/soc/imx/Kconfig      |   7 ++
>>>>>   drivers/soc/imx/Makefile     |   1 +
>>>>>   drivers/soc/imx/soc-imx-sc.c | 220
>>>>> +++++++++++++++++++++++++++++++++++++++++++
>>>>>   3 files changed, 228 insertions(+)  create mode 100644
>>>>> drivers/soc/imx/soc-imx-sc.c
>>>>
>>>> Rather than creating a new driver, please take a look at Abel's
>>>> generic
>>>> i.MX8 SoC driver, and see if it can be extended to cover i.MX8QXP.
>>
>> Got it, I didn't notice that this patch bas been accepted, I will redo the patch
>> based on it, thanks.
> 
> I have sent the new patch set to support i.MX8QXP SoC revision based on generic i.MX8
> SoC driver, however, the Kconfig modification is NOT good, it may break i.MX8MQ if IMX_SCU
> is NOT enabled, although we can add some warp function for SCU firmware API call to fix it,
> but after further thought and discussion with Dong Aisheng, I think we may need to roll back to
> use this patch series to create a new SoC driver dedicated for i.MX8 SoCs
> with system controller inside, such as i.MX8QXP, i.MX8QM etc., the reason are as below:
> 
> For i.MX8MQ/i.MX8MM:
> 	1. SoC driver does NOT depends on i.MX SCU firmware, so no need to use platform driver
> 	     probe model, just device_init phase call is good enough;
> 	2. The SoC driver no need to depends on IMX_SCU, so it can be always built in, no need to
> 	     check IMX_SCU config;
> 	3. The fuse check for CPU speed grading, HDCP status, NoC settings etc. could be added to this driver,
> 	    but they are ONLY for i.MX8MQ/i.MX8MM etc..
> For i.MX8QXP/i.MX8QM:
> 	1. SoC driver MUST depends on IMX_SCU;
> 	2. MUST use platform model to support defer probe;
> 	3. No fuse check for CPU speed grading.
> 
> So, I guess the reused code for i.MX8MQ and i.MX8QXP is ONLY those part of creating SoC id device node (less than
> 30% I think), all other functions are implemented in total different ways, that is why I created the imx_sc_soc driver
> in this patch series, so do you think we can add new SoC driver for i.MX8 SoC with SCU inside? Putting 2 different architecture
> SoCs' driver into 1 file looks like NOT making enough sense.

+1 for separate SOC driver. The 8mq/8mm and 8qm/8qxp families are very 
different, they just happen to share the imx8 prefix.

It makes sense to allow people to compile one without the other and this 
is easier with distinct SOC drivers.

--
Regards,
Leonard