netdev - Re: [EXTERNAL]Re: [PATCH net-next 2/2] net: pse-pd: Add Si3474 PSE controller driver

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: <c23d2b2e-6ebb-4a44-bd23-5a66b2cb4e38@adtran.com>
Date: Thu, 15 May 2025 15:58:10 +0000
From: Piotr Kubik <piotr.kubik@...ran.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, Oleksij Rempel
	<o.rempel@...gutronix.de>, Kory Maincent <kory.maincent@...tlin.com>, Andrew
 Lunn <andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>, Eric
 Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
	<krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [EXTERNAL]Re: [PATCH net-next 2/2] net: pse-pd: Add Si3474 PSE
 controller driver

On 5/15/25 17:40, Krzysztof Kozlowski wrote:
> On 15/05/2025 17:20, Piotr Kubik wrote:
>> Thanks Krzysztof for your review,
>>
>>> On 13/05/2025 00:06, Piotr Kubik wrote:
>>>> +/* Parse pse-pis subnode into chan array of si3474_priv */
>>>> +static int si3474_get_of_channels(struct si3474_priv *priv)
>>>> +{
>>>> +  struct device_node *pse_node, *node;
>>>> +  struct pse_pi *pi;
>>>> +  u32 pi_no, chan_id;
>>>> +  s8 pairset_cnt;
>>>> +  s32 ret = 0;
>>>> +
>>>> +  pse_node = of_get_child_by_name(priv->np, "pse-pis");
>>>> +  if (!pse_node) {
>>>> +          dev_warn(&priv->client[0]->dev,
>>>> +                   "Unable to parse DT PSE power interface matrix, no pse-pis node\n");
>>>> +          return -EINVAL;
>>>> +  }
>>>> +
>>>> +  for_each_child_of_node(pse_node, node) {
>>>
>>> Use scoped variant. One cleanup less.
>>
>> good point
>>
>>>
>>>
>>>> +          if (!of_node_name_eq(node, "pse-pi"))
>>>> +                  continue;
>>>
>>> ...
>>>
>>>> +
>>>> +  ret = i2c_smbus_read_byte_data(client, FIRMWARE_REVISION_REG);
>>>> +  if (ret < 0)
>>>> +          return ret;
>>>> +  fw_version = ret;
>>>> +
>>>> +  ret = i2c_smbus_read_byte_data(client, CHIP_REVISION_REG);
>>>> +  if (ret < 0)
>>>> +          return ret;
>>>> +
>>>> +  dev_info(dev, "Chip revision: 0x%x, firmware version: 0x%x\n",
>>>
>>> dev_dbg or just drop. Drivers should be silent on success.
>>
>> Is there any rule for this I'm not aware of?
>> I'd like to know that device is present and what versions it runs just by looking into dmesg.
>> This approach is similar to other drivers, all current PSE drivers log it this way.
>>
> And now I noticed that you already sent it, you got review:
> https://lore.kernel.org/all/6ee047d4-f3de-4c25-aaae-721221dc3003@kernel.org/
>
> and you ignored it completely sending the same again.
>
> Sending the same over and over and asking us to do the same review over
> and over is really waste of our time.
>
> Go back to v1, implement entire review. Then start versioning your patches.
>
> Best regards,
> Krzysztof


I didn't ignore, I replied to your comment, since there was no answer I assumed you agree.
https://lore.kernel.org/all/38b02e2d-7935-4a23-b351-d23941e781b0@adtran.com/

Thanks for a reference and explanation, I'll change it.

/Piotr