From: Dan Scally <djrscally@gmail.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: yong.zhi@intel.com, sakari.ailus@linux.intel.com,
bingbu.cao@intel.com, tian.shu.qiu@intel.com, mchehab@kernel.org,
davem@davemloft.net, robh@kernel.org, gregkh@linuxfoundation.org,
linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
jorhand@linux.microsoft.com, kitakar@gmail.com,
kieran.bingham@ideasonboard.com, dan.carpenter@oracle.com
Subject: Re: [RFC PATCH v2] Add functionality to ipu3-cio2 driver allowing software_node connections to sensors on platforms designed for Windows
Date: Fri, 9 Oct 2020 16:02:28 +0100 [thread overview]
Message-ID: <694f4ce1-76eb-914a-ac81-2be969cc3665@gmail.com> (raw)
In-Reply-To: <20201009124424.GD4077@smile.fi.intel.com>
On 09/10/2020 13:44, Andy Shevchenko wrote:
>> ...I agree with your other email re. turning this into a series and
>> making the additional changes you suggested, so I'll do that too for the v3.
> I forgot to mention module rename as a separate patch. So, something like 6
> (or more) in a series I would expect.
Yeah no problem, I'll do that too.
>
>>>> +// SPDX-License-Identifier: GPL-2.0
>>> Author line perhaps?
>> You mean literally just like /* Authored By: Dan Scally */ or something?
>> OK, no problem.
> * Author: Dan Scally <foo@bar.com>
Okedokey
>>>> +static const struct ipu3_sensor supported_devices[] = {
>>>> + IPU3_SENSOR("INT33BE", "INT33BE:00"),
>>>> + IPU3_SENSOR("OVTI2680", "OVTI2680:00"),
>>>> + IPU3_SENSOR("OVTI5648", "OVTI5648:00")
>>> In such cases please leave comma at the last item as well. Easier to extend w/o
>>> an additional churn.
>>>
>>> On top of that, please avoid putting *instance* names, i.e. the second
>>> parameters in your macro call. What code should do is to take _HID (first
>>> parameter) and call acpi_dev_match_first_dev() or so.
>> Yeah I was originally using the i2c_client's name field (this comes into
>> play during cio2_bridge_reprobe_sensor()) but the matching refused to
>> work using anything but a string literal. Let me take another look at
>> this then.
> I meant that you get an instance name from the first found device, like
>
> char instance_name[I2C...];
>
> adev = first_match_dev();
> if (adev) {
> snprintf(instance_name, ..., acpi_dev_name(adev));
> ...
> } else {
> ...
> }
Ah, yes, this is better than messing around getting the i2c_client - thanks
>> I thought you were hinting that I should use kcalloc in the comments
>> from the last patch to get an array of zero valued entries but I guess I
>> misunderstood - I can just memset these arrays to 0 and assign all but
>> the last entry directly and that seems to work fine, so I'll switch to that.
> The idea behind that any kcalloc() or kzalloc() against a container (whatever
> data structure that has those property arrays) will do it for you. So, I think
> neither kcalloc() nor stack is needed. Instantiate properties directly in the
> arrays of sensor data structure.
Will do
>>>> + endpoint = fwnode_graph_get_next_endpoint(pci_dev->dev.fwnode, NULL);
>>> dev_fwnode()
>> I avoided that thinking there might be a case where a CIO2 device has a
>> fwnode but not endpoints defined for some reason, but I'm not familiar
>> enough to judge whether that situation will ever occur - if it's safe to
>> do it that way then I'll switch it over.
> I meant
> endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(&pci_dev->dev), NULL);
Ah! Of course, I'll make that change too, thanks
next prev parent reply other threads:[~2020-10-09 15:02 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-01 9:33 [RFC PATCH v2] Add functionality to ipu3-cio2 driver allowing software_node connections to sensors on platforms designed for Windows Daniel Scally
2020-10-01 17:37 ` Andy Shevchenko
2020-10-08 21:51 ` Dan Scally
2020-10-09 12:44 ` Andy Shevchenko
2020-10-09 15:02 ` Dan Scally [this message]
2020-10-01 17:50 ` Andy Shevchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=694f4ce1-76eb-914a-ac81-2be969cc3665@gmail.com \
--to=djrscally@gmail.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=bingbu.cao@intel.com \
--cc=dan.carpenter@oracle.com \
--cc=davem@davemloft.net \
--cc=gregkh@linuxfoundation.org \
--cc=jorhand@linux.microsoft.com \
--cc=kieran.bingham@ideasonboard.com \
--cc=kitakar@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=robh@kernel.org \
--cc=sakari.ailus@linux.intel.com \
--cc=tian.shu.qiu@intel.com \
--cc=yong.zhi@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).