diff mbox series

[01/28] media: cec: ch7322: drop of_match_ptr for ID table

Message ID 20230312131318.351173-1-krzysztof.kozlowski@linaro.org
State Accepted
Commit 76024e1e98a01afe50b66cce621a98938e8f6c9f
Headers show
Series [01/28] media: cec: ch7322: drop of_match_ptr for ID table | expand

Commit Message

Krzysztof Kozlowski March 12, 2023, 1:12 p.m. UTC
The driver can match only via the DT table so the table should be always
used and the of_match_ptr does not have any sense (this also allows ACPI
matching via PRP0001, even though it might not be relevant here).

  drivers/media/cec/i2c/ch7322.c:583:34: error: ‘ch7322_of_match’ defined but not used [-Werror=unused-const-variable=]

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/media/cec/i2c/ch7322.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Laurent Pinchart March 12, 2023, 1:44 p.m. UTC | #1
Hi Krzysztof,

Thank you for the patch.

On Sun, Mar 12, 2023 at 02:13:14PM +0100, Krzysztof Kozlowski wrote:
> The driver can match only via the DT table so the table should be always
> used and the of_match_ptr does not have any sense (this also allows ACPI
> matching via PRP0001, even though it might not be relevant here).  This
> also fixes !CONFIG_OF error:
> 
>   drivers/media/i2c/ov2680.c:1149:34: error: ‘ov2680_dt_ids’ defined but not used [-Werror=unused-const-variable=]
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/i2c/ov2680.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
> index 54153bf66bdd..4d8724952c5f 100644
> --- a/drivers/media/i2c/ov2680.c
> +++ b/drivers/media/i2c/ov2680.c
> @@ -1156,7 +1156,7 @@ static struct i2c_driver ov2680_i2c_driver = {
>  	.driver = {
>  		.name  = "ov2680",
>  		.pm = &ov2680_pm_ops,
> -		.of_match_table	= of_match_ptr(ov2680_dt_ids),
> +		.of_match_table	= ov2680_dt_ids,
>  	},
>  	.probe_new	= ov2680_probe,
>  	.remove		= ov2680_remove,
Martin Blumenstingl March 12, 2023, 8:22 p.m. UTC | #2
On Sun, Mar 12, 2023 at 2:13 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> The driver can match only via the DT table so the table should be always
> used and the of_match_ptr does not have any sense (this also allows ACPI
> matching via PRP0001, even though it might not be relevant here).
>
>   drivers/media/cec/platform/meson/ao-cec.c:711:34: error: ‘meson_ao_cec_of_match’ defined but not used [-Werror=unused-const-variable=]
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Hans Verkuil March 13, 2023, 11:46 a.m. UTC | #3
On 12/03/2023 14:12, Krzysztof Kozlowski wrote:
> The driver can match only via the DT table so the table should be always
> used and the of_match_ptr does not have any sense (this also allows ACPI
> matching via PRP0001, even though it might not be relevant here).
> 
>   drivers/media/cec/platform/meson/ao-cec.c:711:34: error: ‘meson_ao_cec_of_match’ defined but not used [-Werror=unused-const-variable=]
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Thanks!

	Hans

> ---
>  drivers/media/cec/platform/meson/ao-cec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/cec/platform/meson/ao-cec.c b/drivers/media/cec/platform/meson/ao-cec.c
> index 6b440f0635d9..223c092dbb6a 100644
> --- a/drivers/media/cec/platform/meson/ao-cec.c
> +++ b/drivers/media/cec/platform/meson/ao-cec.c
> @@ -719,7 +719,7 @@ static struct platform_driver meson_ao_cec_driver = {
>  	.remove  = meson_ao_cec_remove,
>  	.driver  = {
>  		.name = "meson-ao-cec",
> -		.of_match_table = of_match_ptr(meson_ao_cec_of_match),
> +		.of_match_table = meson_ao_cec_of_match,
>  	},
>  };
>
Sean Young March 13, 2023, 2:47 p.m. UTC | #4
On Sun, Mar 12, 2023 at 02:12:56PM +0100, Krzysztof Kozlowski wrote:
> The driver can match only via the DT table so the table should be always
> used and the of_match_ptr does not have any sense (this also allows ACPI
> matching via PRP0001, even though it might not be relevant here).  This
> also fixes !CONFIG_OF error:
> 
>   drivers/media/rc/ir-rx51.c:264:34: error: ‘ir_rx51_match’ defined but not used [-Werror=unused-const-variable=]
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Reviewed-by: Sean Young <sean@mess.org>

Thanks
Sean

> ---
>  drivers/media/rc/ir-rx51.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
> index adbbe639a261..13e81bf8005d 100644
> --- a/drivers/media/rc/ir-rx51.c
> +++ b/drivers/media/rc/ir-rx51.c
> @@ -275,7 +275,7 @@ static struct platform_driver ir_rx51_platform_driver = {
>  	.resume		= ir_rx51_resume,
>  	.driver		= {
>  		.name	= KBUILD_MODNAME,
> -		.of_match_table = of_match_ptr(ir_rx51_match),
> +		.of_match_table = ir_rx51_match,
>  	},
>  };
>  module_platform_driver(ir_rx51_platform_driver);
> -- 
> 2.34.1
Hans Verkuil March 14, 2023, 8:34 a.m. UTC | #5
On 13/03/2023 23:01, Guenter Roeck wrote:
> On Sun, Mar 12, 2023 at 02:12:51PM +0100, Krzysztof Kozlowski wrote:
>> The driver can match only via the DT table so the table should be always
>> used and the of_match_ptr does not have any sense (this also allows ACPI
>> matching via PRP0001, even though it might not be relevant here).
>>
>>   drivers/media/cec/i2c/ch7322.c:583:34: error: ‘ch7322_of_match’ defined but not used [-Werror=unused-const-variable=]
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> 
> FWIW: There is also
> https://patches.linaro.org/project/linux-media/patch/20230215214724.3798917-1-linux@roeck-us.net/

I superseded that patch in patchwork in favor of this patch since this is part of
a larger series.

Krzysztof, please let me know if you will take this series yourself or want me to
make a PR for it.

Regards,

	Hans

> 
> The lack of ACPI support is a real problem and very relevant.
> 
> Guenter
> 
>> ---
>>  drivers/media/cec/i2c/ch7322.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/cec/i2c/ch7322.c b/drivers/media/cec/i2c/ch7322.c
>> index 34fad7123704..3c6e6496a001 100644
>> --- a/drivers/media/cec/i2c/ch7322.c
>> +++ b/drivers/media/cec/i2c/ch7322.c
>> @@ -589,7 +589,7 @@ MODULE_DEVICE_TABLE(of, ch7322_of_match);
>>  static struct i2c_driver ch7322_i2c_driver = {
>>  	.driver = {
>>  		.name = "ch7322",
>> -		.of_match_table = of_match_ptr(ch7322_of_match),
>> +		.of_match_table = ch7322_of_match,
>>  	},
>>  	.probe_new	= ch7322_probe,
>>  	.remove		= ch7322_remove,
Krzysztof Kozlowski March 14, 2023, 4:32 p.m. UTC | #6
On 14/03/2023 09:34, Hans Verkuil wrote:
> On 13/03/2023 23:01, Guenter Roeck wrote:
>> On Sun, Mar 12, 2023 at 02:12:51PM +0100, Krzysztof Kozlowski wrote:
>>> The driver can match only via the DT table so the table should be always
>>> used and the of_match_ptr does not have any sense (this also allows ACPI
>>> matching via PRP0001, even though it might not be relevant here).
>>>
>>>   drivers/media/cec/i2c/ch7322.c:583:34: error: ‘ch7322_of_match’ defined but not used [-Werror=unused-const-variable=]
>>>
>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>
>> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>>
>> FWIW: There is also
>> https://patches.linaro.org/project/linux-media/patch/20230215214724.3798917-1-linux@roeck-us.net/
> 
> I superseded that patch in patchwork in favor of this patch since this is part of
> a larger series.

I am personally fan of FIFO, so Guenter's patch should go in.

> 
> Krzysztof, please let me know if you will take this series yourself or want me to
> make a PR for it.

I prefer if you take entire patchset.


Best regards,
Krzysztof
Krzysztof Kozlowski May 12, 2023, 4:35 p.m. UTC | #7
On 12/03/2023 14:12, Krzysztof Kozlowski wrote:
> The driver can match only via the DT table so the table should be always
> used and the of_match_ptr does not have any sense (this also allows ACPI
> matching via PRP0001, even though it might not be relevant here).
> 
>   drivers/media/cec/i2c/ch7322.c:583:34: error: ‘ch7322_of_match’ defined but not used [-Werror=unused-const-variable=]
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---

Hans, Sakari,

Can you pick up the patchset? There was positive feedback:
https://patchwork.linuxtv.org/project/linux-media/patch/20230312131318.351173-1-krzysztof.kozlowski@linaro.org/

but it seems it was not applied.

Best regards,
Krzysztof
Hans Verkuil May 13, 2023, 9:57 a.m. UTC | #8
On 12/05/2023 18:35, Krzysztof Kozlowski wrote:
> On 12/03/2023 14:12, Krzysztof Kozlowski wrote:
>> The driver can match only via the DT table so the table should be always
>> used and the of_match_ptr does not have any sense (this also allows ACPI
>> matching via PRP0001, even though it might not be relevant here).
>>
>>   drivers/media/cec/i2c/ch7322.c:583:34: error: ‘ch7322_of_match’ defined but not used [-Werror=unused-const-variable=]
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
> 
> Hans, Sakari,
> 
> Can you pick up the patchset? There was positive feedback:
> https://patchwork.linuxtv.org/project/linux-media/patch/20230312131318.351173-1-krzysztof.kozlowski@linaro.org/
> 
> but it seems it was not applied.

I see it is delegated to Sakari in patchwork and marked Under Review, but I don't
see a corresponding pull request for this series.

Sakari, did something go wrong?

Regards,

	Hans

> 
> Best regards,
> Krzysztof
>
Guenter Roeck June 21, 2023, 6:06 p.m. UTC | #9
On Sun, Mar 12, 2023 at 02:12:51PM +0100, Krzysztof Kozlowski wrote:
> The driver can match only via the DT table so the table should be always
> used and the of_match_ptr does not have any sense (this also allows ACPI
> matching via PRP0001, even though it might not be relevant here).
> 
>   drivers/media/cec/i2c/ch7322.c:583:34: error: ‘ch7322_of_match’ defined but not used [-Werror=unused-const-variable=]
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

What happened wit hthis patch ? I still don't see it applied.

FWIW:

Reviewed-and-tested-by: Guenter Roeck <linux@roeck-us.net>

Thanks,
Guenter

> ---
>  drivers/media/cec/i2c/ch7322.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/cec/i2c/ch7322.c b/drivers/media/cec/i2c/ch7322.c
> index 34fad7123704..3c6e6496a001 100644
> --- a/drivers/media/cec/i2c/ch7322.c
> +++ b/drivers/media/cec/i2c/ch7322.c
> @@ -589,7 +589,7 @@ MODULE_DEVICE_TABLE(of, ch7322_of_match);
>  static struct i2c_driver ch7322_i2c_driver = {
>  	.driver = {
>  		.name = "ch7322",
> -		.of_match_table = of_match_ptr(ch7322_of_match),
> +		.of_match_table = ch7322_of_match,
>  	},
>  	.probe_new	= ch7322_probe,
>  	.remove		= ch7322_remove,
> -- 
> 2.34.1
>
Hans Verkuil July 13, 2023, 7:07 a.m. UTC | #10
On 13/07/2023 08:46, Krzysztof Kozlowski wrote:
> On 19/06/2023 16:13, Krzysztof Kozlowski wrote:
>> On 25/05/2023 15:19, Sakari Ailus wrote:
>>> Hi folks,
>>>
>>> On Thu, May 25, 2023 at 03:40:04PM +0300, Sakari Ailus wrote:
>>>> Hi Hans,
>>>>
>>>> On Sat, May 13, 2023 at 11:57:33AM +0200, Hans Verkuil wrote:
>>>>> On 12/05/2023 18:35, Krzysztof Kozlowski wrote:
>>>>>> On 12/03/2023 14:12, Krzysztof Kozlowski wrote:
>>>>>>> The driver can match only via the DT table so the table should be always
>>>>>>> used and the of_match_ptr does not have any sense (this also allows ACPI
>>>>>>> matching via PRP0001, even though it might not be relevant here).
>>>>>>>
>>>>>>>   drivers/media/cec/i2c/ch7322.c:583:34: error: ‘ch7322_of_match’ defined but not used [-Werror=unused-const-variable=]
>>>>>>>
>>>>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>>>> ---
>>>>>>
>>>>>> Hans, Sakari,
>>>>>>
>>>>>> Can you pick up the patchset? There was positive feedback:
>>>>>> https://patchwork.linuxtv.org/project/linux-media/patch/20230312131318.351173-1-krzysztof.kozlowski@linaro.org/
>>>>>>
>>>>>> but it seems it was not applied.
>>>>>
>>>>> I see it is delegated to Sakari in patchwork and marked Under Review, but I don't
>>>>> see a corresponding pull request for this series.
>>>>>
>>>>> Sakari, did something go wrong?
>>>>
>>>> I spotted this as Hans notified me in IRC, I wasn't cc'd. Apologies for
>>>> this --- I intended to take these but I think I must have missed something
>>>> important in the process. I'll take them now.
>>>>
>>>> Thanks.
>>>
>>> This no longer applied cleanly. Mostly there was fuzz near .of_match_table
>>> changes as probe_new (and remove_new?) changes have been recently merged.
>>> There were other issues as well, I marked a few patches in the set "not
>>> applicable" as other patches had already done equivalent changes earlier.
>>>
>>> There were substance-changing changes in the 20th patch, replied to the
>>> disuccsion there.
>>>
>>> I've pushed the result here and intend to send PR to Mauro soon if there
>>> are no issues:
>>>
>>> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=of-match-ptr>
>>
>> One month later, I still don't see this set in the linux-next.  What's
>> happening here?
>>
> 
> I don't think this was merged in v6.5-rc1. It's not in linux-next,
> either. Another month passed...

I'm really hopeful it will be merged very soon. As you may have noticed (!) we
have problems with maintaining the media subsystem (too many patches, not enough
maintainers). We had a meeting about that a few weeks ago and are working on
it.

The core problem for this particular series was that the PR containing this series
was posted much too late (as in three months too late!), and it came in too late
for 6.5.

Regards,

	Hans
Sakari Ailus July 25, 2023, 10:51 a.m. UTC | #11
Hi Krzysztof,

On Thu, Jul 13, 2023 at 08:46:58AM +0200, Krzysztof Kozlowski wrote:
> On 19/06/2023 16:13, Krzysztof Kozlowski wrote:
> > On 25/05/2023 15:19, Sakari Ailus wrote:
> >> Hi folks,
> >>
> >> On Thu, May 25, 2023 at 03:40:04PM +0300, Sakari Ailus wrote:
> >>> Hi Hans,
> >>>
> >>> On Sat, May 13, 2023 at 11:57:33AM +0200, Hans Verkuil wrote:
> >>>> On 12/05/2023 18:35, Krzysztof Kozlowski wrote:
> >>>>> On 12/03/2023 14:12, Krzysztof Kozlowski wrote:
> >>>>>> The driver can match only via the DT table so the table should be always
> >>>>>> used and the of_match_ptr does not have any sense (this also allows ACPI
> >>>>>> matching via PRP0001, even though it might not be relevant here).
> >>>>>>
> >>>>>>   drivers/media/cec/i2c/ch7322.c:583:34: error: ‘ch7322_of_match’ defined but not used [-Werror=unused-const-variable=]
> >>>>>>
> >>>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >>>>>> ---
> >>>>>
> >>>>> Hans, Sakari,
> >>>>>
> >>>>> Can you pick up the patchset? There was positive feedback:
> >>>>> https://patchwork.linuxtv.org/project/linux-media/patch/20230312131318.351173-1-krzysztof.kozlowski@linaro.org/
> >>>>>
> >>>>> but it seems it was not applied.
> >>>>
> >>>> I see it is delegated to Sakari in patchwork and marked Under Review, but I don't
> >>>> see a corresponding pull request for this series.
> >>>>
> >>>> Sakari, did something go wrong?
> >>>
> >>> I spotted this as Hans notified me in IRC, I wasn't cc'd. Apologies for
> >>> this --- I intended to take these but I think I must have missed something
> >>> important in the process. I'll take them now.
> >>>
> >>> Thanks.
> >>
> >> This no longer applied cleanly. Mostly there was fuzz near .of_match_table
> >> changes as probe_new (and remove_new?) changes have been recently merged.
> >> There were other issues as well, I marked a few patches in the set "not
> >> applicable" as other patches had already done equivalent changes earlier.
> >>
> >> There were substance-changing changes in the 20th patch, replied to the
> >> disuccsion there.
> >>
> >> I've pushed the result here and intend to send PR to Mauro soon if there
> >> are no issues:
> >>
> >> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=of-match-ptr>
> > 
> > One month later, I still don't see this set in the linux-next.  What's
> > happening here?
> > 
> 
> I don't think this was merged in v6.5-rc1. It's not in linux-next,
> either. Another month passed...

These are now in the media-stage tree
<URL:https://git.linuxtv.org/media_stage.git/log/>. Assuming nothing goes
wrong, these should end up in Linus's tree for 6.6.

The issues related to media tree maintenance are being addressed as we're
changing the process how the tree is maintained. The patches will
eventually get in still, also expect this to improve in the future.
Krzysztof Kozlowski July 26, 2023, 8:20 a.m. UTC | #12
On 25/07/2023 12:51, Sakari Ailus wrote:
>>
>> I don't think this was merged in v6.5-rc1. It's not in linux-next,
>> either. Another month passed...
> 
> These are now in the media-stage tree
> <URL:https://git.linuxtv.org/media_stage.git/log/>. Assuming nothing goes
> wrong, these should end up in Linus's tree for 6.6.

Great, thank you!

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/drivers/media/cec/i2c/ch7322.c b/drivers/media/cec/i2c/ch7322.c
index 34fad7123704..3c6e6496a001 100644
--- a/drivers/media/cec/i2c/ch7322.c
+++ b/drivers/media/cec/i2c/ch7322.c
@@ -589,7 +589,7 @@  MODULE_DEVICE_TABLE(of, ch7322_of_match);
 static struct i2c_driver ch7322_i2c_driver = {
 	.driver = {
 		.name = "ch7322",
-		.of_match_table = of_match_ptr(ch7322_of_match),
+		.of_match_table = ch7322_of_match,
 	},
 	.probe_new	= ch7322_probe,
 	.remove		= ch7322_remove,