Re: [PATCH v11 04/10] soc: mediatek: Add multiple step bus protection control

[Matthias Brugger]

On 13/02/2020 03:15, Weiyi Lu wrote:
> On Wed, 2020-02-12 at 10:23 +0100, Matthias Brugger wrote:
>>
>> On 12/02/2020 03:55, Weiyi Lu wrote:
>>> On Tue, 2020-02-11 at 18:49 +0100, Matthias Brugger wrote:
>>>>
>>>> On 20/12/2019 04:45, Weiyi Lu wrote:
>>>>> Both MT8183 & MT6765 have more control steps of bus protection
>>>>> than previous project. And there add more bus protection registers
>>>>> reside at infracfg & smi-common. Also add new APIs for multiple
>>>>> step bus protection control with more customized arguments.
>>>>> And then use bp_table for bus protection of all compatibles,
>>>>> instead of mixing bus_prot_mask and bus_prot_reg_update.
>>>>>
>>>>> Signed-off-by: Weiyi Lu <weiyi.lu@xxxxxxxxxxxx>
>>>>> ---
>>>>>  drivers/soc/mediatek/Makefile         |   2 +-
>>>>>  drivers/soc/mediatek/mtk-scpsys-ext.c | 101 +++++++++++++++++++++++++++++
>>>>>  drivers/soc/mediatek/mtk-scpsys.c     | 117 +++++++++++++++++++++-------------
>>>>>  drivers/soc/mediatek/scpsys-ext.h     |  67 +++++++++++++++++++
>>>>>  4 files changed, 240 insertions(+), 47 deletions(-)
>>>>>  create mode 100644 drivers/soc/mediatek/mtk-scpsys-ext.c
>>>>>  create mode 100644 drivers/soc/mediatek/scpsys-ext.h
>>>>>
>>>>> diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
>>>>> index b017330..b442be9 100644
>>>>> --- a/drivers/soc/mediatek/Makefile
>>>>> +++ b/drivers/soc/mediatek/Makefile
>>>>> @@ -1,5 +1,5 @@
>>>>>  # SPDX-License-Identifier: GPL-2.0-only
>>>>>  obj-$(CONFIG_MTK_CMDQ) += mtk-cmdq-helper.o
>>>>> -obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
>>>>> +obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o mtk-scpsys-ext.o
>>>>
>>>> It seems that we would need another patch which get's rid of the mtk-infracfg
>>>> first and then add stuff like the possibility to have different steps.
>>>>
>>>
>>> Actually I have a PATCH 05/11 to remove the mtk-infracfg.
>>> In this patch, I have some changes, like calling
>>> mtk_scpsys_ext_set_bus_protection(...) instead of
>>> mtk_infracfg_set_bus_protection(...) in scpsys_bus_protect_enable(...)
>>> and replacing bus_prot_mask by bp_table.
>>> I thought I should introduce the new method first and remove useless one
>>> later. What do you think?
>>
>> Yes, but first patch would be a step to get rid of mtk-infracfg and a second
>> patch add bp_table and the like.
>>
> 
> OK, I'll split into such sequence.
> 
>>>
>>>>>  obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
>>>>>  obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
>>>>> diff --git a/drivers/soc/mediatek/mtk-scpsys-ext.c b/drivers/soc/mediatek/mtk-scpsys-ext.c
>>>>> new file mode 100644
>>>>> index 0000000..df402ac
>>>>> --- /dev/null
>>>>> +++ b/drivers/soc/mediatek/mtk-scpsys-ext.c
>>>>
>>>> I'm not quite sure why we should put this into a new file. I suppose the
>>>> rational behind it is the fact that we access other blocks through regmap.
>>>>
>>>
>>> Yes, those operation are accross infracfg and smi-common so we put these
>>> into new files.
>>
>> Are you exepct other drivers to use this functions? If not I will have to think
>> again, but I don't see any reason to put it into a new file.
>>
> 
> I thought no other driver would use these functions unless there will
> have drivers want to take over the bus protection process by themselves.
> 
> Do you prefer just putting these functions into mtk-scpsys.c?
> 

Yes I'd prefer that.

Regards,
Matthias