| Message ID | 20230310214553.275450-2-krzysztof.kozlowski@linaro.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [1/9] regulator: lp872x: Mark OF related data as maybe unused | expand |
Hi Krzysztof, Thank you for the patch. On Fri, Mar 10, 2023 at 10:45:46PM +0100, Krzysztof Kozlowski wrote: > The driver can be compile tested with !CONFIG_OF making certain data > unused: > > drivers/regulator/max20086-regulator.c:289:34: error: ‘max20086_dt_ids’ defined but not used [-Werror=unused-const-variable=] > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > --- > drivers/regulator/max20086-regulator.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/regulator/max20086-regulator.c b/drivers/regulator/max20086-regulator.c > index b8bf76c170fe..c98a72f43935 100644 > --- a/drivers/regulator/max20086-regulator.c > +++ b/drivers/regulator/max20086-regulator.c > @@ -286,7 +286,7 @@ static const struct i2c_device_id max20086_i2c_id[] = { > > MODULE_DEVICE_TABLE(i2c, max20086_i2c_id); > > -static const struct of_device_id max20086_dt_ids[] = { > +static const struct of_device_id max20086_dt_ids[] __maybe_unused = { The following change would also work, as the of_match_table field of struct device_driver isn't conditioned by CONFIG_OF: diff --git a/drivers/regulator/max20086-regulator.c b/drivers/regulator/max20086-regulator.c index b8bf76c170fe..ad92f84b4abb 100644 --- a/drivers/regulator/max20086-regulator.c +++ b/drivers/regulator/max20086-regulator.c @@ -320,7 +320,7 @@ MODULE_DEVICE_TABLE(of, max20086_dt_ids); static struct i2c_driver max20086_regulator_driver = { .driver = { .name = "max20086", - .of_match_table = of_match_ptr(max20086_dt_ids), + .of_match_table = max20086_dt_ids, }, .probe_new = max20086_i2c_probe, .id_table = max20086_i2c_id, Your patch should reduce the module size without any real drawback as far as I can see, so that's probably best. I'm fine with either approach, so Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > { > .compatible = "maxim,max20086", > .data = &(const struct max20086_chip_info) {
On 12/03/2023 10:42, Laurent Pinchart wrote: > Hi Krzysztof, > > Thank you for the patch. > > On Fri, Mar 10, 2023 at 10:45:46PM +0100, Krzysztof Kozlowski wrote: >> The driver can be compile tested with !CONFIG_OF making certain data >> unused: >> >> drivers/regulator/max20086-regulator.c:289:34: error: ‘max20086_dt_ids’ defined but not used [-Werror=unused-const-variable=] >> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> --- >> drivers/regulator/max20086-regulator.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/regulator/max20086-regulator.c b/drivers/regulator/max20086-regulator.c >> index b8bf76c170fe..c98a72f43935 100644 >> --- a/drivers/regulator/max20086-regulator.c >> +++ b/drivers/regulator/max20086-regulator.c >> @@ -286,7 +286,7 @@ static const struct i2c_device_id max20086_i2c_id[] = { >> >> MODULE_DEVICE_TABLE(i2c, max20086_i2c_id); >> >> -static const struct of_device_id max20086_dt_ids[] = { >> +static const struct of_device_id max20086_dt_ids[] __maybe_unused = { > > The following change would also work, as the of_match_table field of > struct device_driver isn't conditioned by CONFIG_OF: > > diff --git a/drivers/regulator/max20086-regulator.c b/drivers/regulator/max20086-regulator.c > index b8bf76c170fe..ad92f84b4abb 100644 > --- a/drivers/regulator/max20086-regulator.c > +++ b/drivers/regulator/max20086-regulator.c > @@ -320,7 +320,7 @@ MODULE_DEVICE_TABLE(of, max20086_dt_ids); > static struct i2c_driver max20086_regulator_driver = { > .driver = { > .name = "max20086", > - .of_match_table = of_match_ptr(max20086_dt_ids), > + .of_match_table = max20086_dt_ids, > }, > .probe_new = max20086_i2c_probe, > .id_table = max20086_i2c_id, > > Your patch should reduce the module size without any real drawback as > far as I can see, so that's probably best. I'm fine with either > approach, so I know it would work. If you check all my patches you see both patterns used depending on the needs: https://lore.kernel.org/all/?q=f%3Akrzysztof+of_device_id (~100 patches so far) The point is that here the device can actually match via ID table, so OF table could stay optional. I don't think PRP0001 is relevant here, thus I proposed to keep OF table optional. Different folks have different opinion on that, so if general consensus is that availability of OF ID table (for PRP0001) is preferred, I can rework the patch towards it. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks Best regards, Krzysztof
diff --git a/drivers/regulator/max20086-regulator.c b/drivers/regulator/max20086-regulator.c index b8bf76c170fe..c98a72f43935 100644 --- a/drivers/regulator/max20086-regulator.c +++ b/drivers/regulator/max20086-regulator.c @@ -286,7 +286,7 @@ static const struct i2c_device_id max20086_i2c_id[] = { MODULE_DEVICE_TABLE(i2c, max20086_i2c_id); -static const struct of_device_id max20086_dt_ids[] = { +static const struct of_device_id max20086_dt_ids[] __maybe_unused = { { .compatible = "maxim,max20086", .data = &(const struct max20086_chip_info) {
The driver can be compile tested with !CONFIG_OF making certain data unused: drivers/regulator/max20086-regulator.c:289:34: error: ‘max20086_dt_ids’ defined but not used [-Werror=unused-const-variable=] Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- drivers/regulator/max20086-regulator.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)