linux-kernel - Re: [PATCH v3 RESEND] media: i2c: Add OV05C10 camera sensor 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: <163655af-2a3d-4489-ac7a-4ee31d3980e2@amd.com>
Date: Mon, 23 Jun 2025 19:28:46 -0400
From: "Nirujogi, Pratap" <pnirujog@....com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: Pratap Nirujogi <pratap.nirujogi@....com>, mchehab@...nel.org,
 sakari.ailus@...ux.intel.com, hverkuil@...all.nl,
 bryan.odonoghue@...aro.org, krzk@...nel.org, dave.stevenson@...pberrypi.com,
 hdegoede@...hat.com, jai.luthra@...asonboard.com,
 tomi.valkeinen@...asonboard.com, linux-media@...r.kernel.org,
 linux-kernel@...r.kernel.org, benjamin.chan@....com, bin.du@....com,
 grosikop@....com, king.li@....com, dantony@....com, vengutta@....com,
 Svetoslav.Stoilov@....com, Yana.Zheleva@....com,
 Mehdi Djait <mehdi.djait@...ux.intel.com>
Subject: Re: [PATCH v3 RESEND] media: i2c: Add OV05C10 camera sensor driver

Hi Laurent,

On 6/23/2025 6:05 PM, Laurent Pinchart wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> (CC'ing Mehdi)
> 
> On Mon, Jun 23, 2025 at 05:51:48PM -0400, Nirujogi, Pratap wrote:
>> On 6/16/2025 6:49 PM, Nirujogi, Pratap wrote:
>>>>> +static int ov05c10_probe(struct i2c_client *client)
>>>>> +{
>>>>> +     struct ov05c10 *ov05c10;
>>>>> +     u32 clkfreq;
>>>>> +     int ret;
>>>>> +
>>>>> +     ov05c10 = devm_kzalloc(&client->dev, sizeof(*ov05c10),
>>>>> GFP_KERNEL);
>>>>> +     if (!ov05c10)
>>>>> +             return -ENOMEM;
>>>>> +
>>>>> +     struct fwnode_handle *fwnode = dev_fwnode(&client->dev);
>>>>> +
>>>>> +     ret = fwnode_property_read_u32(fwnode, "clock-frequency",
>>>>> &clkfreq);
>>>>> +     if (ret)
>>>>> +             return  dev_err_probe(&client->dev, -EINVAL,
>>>>> +                                   "fail to get clock freq\n");
>>>>
>>>> Let's try to land
>>>> https://lore.kernel.org/linux-media/20250521104115.176950-1-
>>>> mehdi.djait@...ux.intel.com/
>>>> and replace the code above with devm_v4l2_sensor_clk_get().
>>>>
>>> Ok, we will verify on our side.
>>
>> We tried using devm_v4l2_sensor_clk_get() and found its required to add
>> support for software_node to make it work with this driver.
> 
> Why is that ?
> 
Its because the i2c_client device is initialized with swnode in the 
x86/platform driver.

https://github.com/torvalds/linux/blob/master/drivers/platform/x86/amd/amd_isp4.c#L235

Thanks,
Pratap


>> Please refer
>> the changes below and let us know if these should be submitted as a
>> separate patch.
> 
> Mehdi, do you have any comment ?
> 
>> ---
>> @@ -645,16 +645,16 @@ struct clk *devm_v4l2_sensor_clk_get(struct device
>> *dev, const char *id)
>>           const char *clk_id __free(kfree) = NULL;
>>           struct clk_hw *clk_hw;
>>           struct clk *clk;
>> -       bool acpi_node;
>> +       bool is_node;
>>           u32 rate;
>>           int ret;
>>
>>           clk = devm_clk_get_optional(dev, id);
>>           ret = device_property_read_u32(dev, "clock-frequency", &rate);
>> -       acpi_node = is_acpi_node(dev_fwnode(dev));
>> +       is_node = is_acpi_node(dev_fwnode(dev)) ||
>> is_software_node(dev_fwnode(dev));
>>
>>           if (clk) {
>> -               if (!ret && acpi_node) {
>> +               if (!ret && is_node) {
>>                           ret = clk_set_rate(clk, rate);
>>                           if (ret) {
>>                                   dev_err(dev, "Failed to set clock rate:
>> %u\n",
>> @@ -668,7 +668,7 @@ struct clk *devm_v4l2_sensor_clk_get(struct device
>> *dev, const char *id)
>>           if (ret)
>>                   return ERR_PTR(ret);
>>
>> -       if (!IS_ENABLED(CONFIG_COMMON_CLK) || !acpi_node)
>> +       if (!IS_ENABLED(CONFIG_COMMON_CLK) || !is_node)
>>                   return ERR_PTR(-ENOENT);
>>
>>           if (!id) {
>> ----
> 
> --
> Regards,
> 
> Laurent Pinchart