linux-kernel - Re: [PATCH 3/4] memory: mtk-smi: mt8188: Add SMI clamp function

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <dc8c3bb0-e462-4286-9060-c54fcc10c186@kernel.org>
Date: Tue, 29 Oct 2024 07:32:53 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Friday Yang (杨阳) <Friday.Yang@...iatek.com>,
 "robh@...nel.org" <robh@...nel.org>,
 "matthias.bgg@...il.com" <matthias.bgg@...il.com>,
 "conor+dt@...nel.org" <conor+dt@...nel.org>,
 AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Cc: "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 Yong Wu (吴勇) <Yong.Wu@...iatek.com>,
 "linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
 Project_Global_Chrome_Upstream_Group
 <Project_Global_Chrome_Upstream_Group@...iatek.com>,
 "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
 "linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 3/4] memory: mtk-smi: mt8188: Add SMI clamp function

On 24/10/2024 03:29, Friday Yang (杨阳) wrote:
> On Wed, 2024-08-21 at 11:00 +0200, Krzysztof Kozlowski wrote:
>>  	 
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>  On 21/08/2024 10:26, friday.yang wrote:
>>> In order to avoid handling glitch signal when MTCMOS on/off, SMI
>> need
>>> clamp and reset operation. Parse power reset settings for LARBs
>> which
>>> need to reset. Register genpd callback for SMI LARBs and apply
>> reset
>>> operations in the callback.
>>>
>>> Signed-off-by: friday.yang <friday.yang@...iatek.com>
>>> ---
>>>  drivers/memory/mtk-smi.c | 148
>> ++++++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 146 insertions(+), 2 deletions(-)
>>>
>>
>> ...
>>
>>> +
>>> +static int mtk_smi_larb_parse_reset_info(struct mtk_smi_larb
>> *larb)
>>> +{
>>> +struct device_node *reset_node;
>>> +struct device *dev = larb->dev;
>>> +int ret;
>>> +
>>> +/* only larb with "resets" need to get reset setting */
>>> +reset_node = of_parse_phandle(dev->of_node, "resets", 0);
>>
>> Nope, you do not parse rasets.
> 
> 1.If I need to use the Linux reset control framework, 'resets' is the
> required property.

And you never parse it directly. Find me existing examples of this, if
you disagree.

> 2.'reset-names' give the list of reset signal name strings sorted in
> the same order as the 'resets' property. SMI driver will use 'reset-
> names' to match reset signal names with reset specifiers.

?

> 3.Not all SMI larbs need to apply reset operations, only SMI larbs
> which may have bus glitch issues need this. Just to confirm if this
> larb support reset function.

?

Really, read kernel API about reset framework first.

> 
>>
>>> +if (!reset_node)
>>> +return 0;
>>> +of_node_put(reset_node);
>>> +
>>> +larb->rst_con = devm_reset_control_get(dev, "larb_rst");
>>
>> Where are the bindings? Why do you add undocumented properties? How
>> possible this passes dtbs_check???
>>
> 
> This is not the new added property in SMI larb DT node.
> It is the reset signal name which is inclued in 'reset-names'.
> Just like this:

$ git grep larb_rst
No such property, so how this could be "not the new added"?

If you keep responding with some random or irrelevant responses, this
won't work. This is really poor way to upstream. I suggest first perform
extensive internal review which would point out all such trivial issues.



Best regards,
Krzysztof