| Message ID | 20230511175446.282041-1-krzysztof.kozlowski@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RESEND] drm/i915: constify pointers to hwmon_channel_info | expand |
On Thu, 11 May 2023, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > Statically allocated array of pointers to hwmon_channel_info can be made > const for safety. > > Acked-by: Jani Nikula <jani.nikula@intel.com> > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> FYI we'll merge this once we've done a backmerge to get the hwmon changes to our tree. BR, Jani. > --- > drivers/gpu/drm/i915/i915_hwmon.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c > index 8e7dccc8d3a0..e99e8c97ef01 100644 > --- a/drivers/gpu/drm/i915/i915_hwmon.c > +++ b/drivers/gpu/drm/i915/i915_hwmon.c > @@ -267,7 +267,7 @@ static const struct attribute_group *hwm_groups[] = { > NULL > }; > > -static const struct hwmon_channel_info *hwm_info[] = { > +static const struct hwmon_channel_info * const hwm_info[] = { > HWMON_CHANNEL_INFO(in, HWMON_I_INPUT), > HWMON_CHANNEL_INFO(power, HWMON_P_MAX | HWMON_P_RATED_MAX | HWMON_P_CRIT), > HWMON_CHANNEL_INFO(energy, HWMON_E_INPUT), > @@ -275,7 +275,7 @@ static const struct hwmon_channel_info *hwm_info[] = { > NULL > }; > > -static const struct hwmon_channel_info *hwm_gt_info[] = { > +static const struct hwmon_channel_info * const hwm_gt_info[] = { > HWMON_CHANNEL_INFO(energy, HWMON_E_INPUT), > NULL > };
On 17/05/2023 11:28, Jani Nikula wrote: > On Thu, 11 May 2023, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: >> Statically allocated array of pointers to hwmon_channel_info can be made >> const for safety. >> >> Acked-by: Jani Nikula <jani.nikula@intel.com> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > FYI we'll merge this once we've done a backmerge to get the hwmon > changes to our tree. There are no dependencies. hwmon changes are already in rc1. Best regards, Krzysztof
On Wed, 17 May 2023, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 17/05/2023 11:28, Jani Nikula wrote: >> On Thu, 11 May 2023, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: >>> Statically allocated array of pointers to hwmon_channel_info can be made >>> const for safety. >>> >>> Acked-by: Jani Nikula <jani.nikula@intel.com> >>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> >> FYI we'll merge this once we've done a backmerge to get the hwmon >> changes to our tree. > > There are no dependencies. hwmon changes are already in rc1. That's what I'm saying, drm-intel-next doesn't have rc1. :) BR, Jani. > > Best regards, > Krzysztof >
Hi Krzysztof, On Thu, May 11, 2023 at 07:54:46PM +0200, Krzysztof Kozlowski wrote: > Statically allocated array of pointers to hwmon_channel_info can be made > const for safety. > > Acked-by: Jani Nikula <jani.nikula@intel.com> > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> Andi
On Wed, 17 May 2023, Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Wed, 17 May 2023, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: >> On 17/05/2023 11:28, Jani Nikula wrote: >>> On Thu, 11 May 2023, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: >>>> Statically allocated array of pointers to hwmon_channel_info can be made >>>> const for safety. >>>> >>>> Acked-by: Jani Nikula <jani.nikula@intel.com> >>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>> >>> FYI we'll merge this once we've done a backmerge to get the hwmon >>> changes to our tree. >> >> There are no dependencies. hwmon changes are already in rc1. > > That's what I'm saying, drm-intel-next doesn't have rc1. :) Pushed to drm-intel-next, thanks for the patch. BR, Jani. > > BR, > Jani. > >> >> Best regards, >> Krzysztof >>
On Thu, 11 May 2023, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > Statically allocated array of pointers to hwmon_channel_info can be made > const for safety. Btw if you want to further make things const, the compound literals defined by HWMON_CHANNEL_INFO() still end up mutable, even if they're only referenced inline using a const pointer. If possible, would be nice to add const there too. BR, Jani. > > Acked-by: Jani Nikula <jani.nikula@intel.com> > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > --- > drivers/gpu/drm/i915/i915_hwmon.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c > index 8e7dccc8d3a0..e99e8c97ef01 100644 > --- a/drivers/gpu/drm/i915/i915_hwmon.c > +++ b/drivers/gpu/drm/i915/i915_hwmon.c > @@ -267,7 +267,7 @@ static const struct attribute_group *hwm_groups[] = { > NULL > }; > > -static const struct hwmon_channel_info *hwm_info[] = { > +static const struct hwmon_channel_info * const hwm_info[] = { > HWMON_CHANNEL_INFO(in, HWMON_I_INPUT), > HWMON_CHANNEL_INFO(power, HWMON_P_MAX | HWMON_P_RATED_MAX | HWMON_P_CRIT), > HWMON_CHANNEL_INFO(energy, HWMON_E_INPUT), > @@ -275,7 +275,7 @@ static const struct hwmon_channel_info *hwm_info[] = { > NULL > }; > > -static const struct hwmon_channel_info *hwm_gt_info[] = { > +static const struct hwmon_channel_info * const hwm_gt_info[] = { > HWMON_CHANNEL_INFO(energy, HWMON_E_INPUT), > NULL > };
On Thu, 25 May 2023, Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Thu, 11 May 2023, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: >> Statically allocated array of pointers to hwmon_channel_info can be made >> const for safety. > > Btw if you want to further make things const, the compound literals > defined by HWMON_CHANNEL_INFO() still end up mutable, even if they're > only referenced inline using a const pointer. If possible, would be nice > to add const there too. Krzysztof, can I persuade you to follow up on this one? ;) With HWMON_CHANNEL_INFO defined like this: #define HWMON_CHANNEL_INFO(stype, ...) \ (&(struct hwmon_channel_info) { \ .type = hwmon_##stype, \ .config = (u32 []) { \ __VA_ARGS__, 0 \ } \ }) and initializers like this all over the kernel: static const struct hwmon_channel_info * const hwm_info[] = { HWMON_CHANNEL_INFO(in, HWMON_I_INPUT), HWMON_CHANNEL_INFO(power, HWMON_P_MAX | HWMON_P_RATED_MAX | HWMON_P_CRIT), HWMON_CHANNEL_INFO(energy, HWMON_E_INPUT), HWMON_CHANNEL_INFO(curr, HWMON_C_CRIT), NULL }; You'll actually end up with *mutable non-const* struct hwmon_channel_info's being allocated in .data sections, and having the const pointers in the arrays point at the mutable stuff. Check with readelf or objdump. To put all of it in .rodata, you'd need to make the compound literals const too: #define HWMON_CHANNEL_INFO(stype, ...) \ - (&(struct hwmon_channel_info) { \ + (&(const struct hwmon_channel_info) { \ .type = hwmon_##stype, \ But I'm not up for going throw all of the use sites to see if they can all be const. BR, Jani. > > BR, > Jani. > >> >> Acked-by: Jani Nikula <jani.nikula@intel.com> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> --- >> drivers/gpu/drm/i915/i915_hwmon.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c >> index 8e7dccc8d3a0..e99e8c97ef01 100644 >> --- a/drivers/gpu/drm/i915/i915_hwmon.c >> +++ b/drivers/gpu/drm/i915/i915_hwmon.c >> @@ -267,7 +267,7 @@ static const struct attribute_group *hwm_groups[] = { >> NULL >> }; >> >> -static const struct hwmon_channel_info *hwm_info[] = { >> +static const struct hwmon_channel_info * const hwm_info[] = { >> HWMON_CHANNEL_INFO(in, HWMON_I_INPUT), >> HWMON_CHANNEL_INFO(power, HWMON_P_MAX | HWMON_P_RATED_MAX | HWMON_P_CRIT), >> HWMON_CHANNEL_INFO(energy, HWMON_E_INPUT), >> @@ -275,7 +275,7 @@ static const struct hwmon_channel_info *hwm_info[] = { >> NULL >> }; >> >> -static const struct hwmon_channel_info *hwm_gt_info[] = { >> +static const struct hwmon_channel_info * const hwm_gt_info[] = { >> HWMON_CHANNEL_INFO(energy, HWMON_E_INPUT), >> NULL >> };
diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c index 8e7dccc8d3a0..e99e8c97ef01 100644 --- a/drivers/gpu/drm/i915/i915_hwmon.c +++ b/drivers/gpu/drm/i915/i915_hwmon.c @@ -267,7 +267,7 @@ static const struct attribute_group *hwm_groups[] = { NULL }; -static const struct hwmon_channel_info *hwm_info[] = { +static const struct hwmon_channel_info * const hwm_info[] = { HWMON_CHANNEL_INFO(in, HWMON_I_INPUT), HWMON_CHANNEL_INFO(power, HWMON_P_MAX | HWMON_P_RATED_MAX | HWMON_P_CRIT), HWMON_CHANNEL_INFO(energy, HWMON_E_INPUT), @@ -275,7 +275,7 @@ static const struct hwmon_channel_info *hwm_info[] = { NULL }; -static const struct hwmon_channel_info *hwm_gt_info[] = { +static const struct hwmon_channel_info * const hwm_gt_info[] = { HWMON_CHANNEL_INFO(energy, HWMON_E_INPUT), NULL };