| Message ID | 20231113112344.719-1-hkallweit1@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | remove I2C_CLASS_DDC support | expand |
On 13.11.2023 13:17, Jani Nikula wrote: > On Mon, 13 Nov 2023, Heiner Kallweit <hkallweit1@gmail.com> wrote: >> After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in >> olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. >> Class-based device auto-detection is a legacy mechanism and shouldn't >> be used in new code. So we can remove this class completely now. > > So this is copy-pasted to all commits and the cover letter, but please > do explain why there are no functional changes here (or are there?), > without me having to go through the i2c stack and try to find the > commits alluded to in "After removal of the legacy ...". > Legacy eeprom driver was marked deprecated 4 yrs ago with: 3079b54aa9a0 ("eeprom: Warn that the driver is deprecated") Now it has been removed with: 0113a99b8a75 ("eeprom: Remove deprecated legacy eeprom driver") Declaration of I2C_CLASS_DDC support is a no-op now, so there's no functional change in this patch. If loaded manually, the legacy eeprom driver exposed the DDC EEPROM to userspace. If this functionality is needed, then now the DDC EEPROM has to be explicitly instantiated using at24. See also: https://docs.kernel.org/i2c/instantiating-devices.html > What does this mean? > > > BR, > Jani. > Heiner > >> >> Preferably this series should be applied via the i2c tree. >> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> >> --- >> drivers/gpu/drm/i915/display/intel_gmbus.c | 1 - >> drivers/gpu/drm/i915/display/intel_sdvo.c | 1 - >> 2 files changed, 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c >> index 40d7b6f3f..e9e4dcf34 100644 >> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c >> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c >> @@ -899,7 +899,6 @@ int intel_gmbus_setup(struct drm_i915_private *i915) >> } >> >> bus->adapter.owner = THIS_MODULE; >> - bus->adapter.class = I2C_CLASS_DDC; >> snprintf(bus->adapter.name, >> sizeof(bus->adapter.name), >> "i915 gmbus %s", gmbus_pin->name); >> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c >> index a636f42ce..5e64d1baf 100644 >> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c >> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c >> @@ -3311,7 +3311,6 @@ intel_sdvo_init_ddc_proxy(struct intel_sdvo_ddc *ddc, >> ddc->ddc_bus = ddc_bus; >> >> ddc->ddc.owner = THIS_MODULE; >> - ddc->ddc.class = I2C_CLASS_DDC; >> snprintf(ddc->ddc.name, I2C_NAME_SIZE, "SDVO %c DDC%d", >> port_name(sdvo->base.port), ddc_bus); >> ddc->ddc.dev.parent = &pdev->dev; >> >
> Preferably this series should be applied via the i2c tree.
Are we in a hurry here, i.e. does it block further development of the
i801 smbus driver? My gut feeling says the patches should rather go via
drm and fbdev trees, but I may be convinced otherwise.
On Mon, 13 Nov 2023, Heiner Kallweit <hkallweit1@gmail.com> wrote: > On 13.11.2023 13:17, Jani Nikula wrote: >> On Mon, 13 Nov 2023, Heiner Kallweit <hkallweit1@gmail.com> wrote: >>> After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in >>> olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. >>> Class-based device auto-detection is a legacy mechanism and shouldn't >>> be used in new code. So we can remove this class completely now. >> >> So this is copy-pasted to all commits and the cover letter, but please >> do explain why there are no functional changes here (or are there?), >> without me having to go through the i2c stack and try to find the >> commits alluded to in "After removal of the legacy ...". >> > Legacy eeprom driver was marked deprecated 4 yrs ago with: > 3079b54aa9a0 ("eeprom: Warn that the driver is deprecated") > Now it has been removed with: > 0113a99b8a75 ("eeprom: Remove deprecated legacy eeprom driver") > > Declaration of I2C_CLASS_DDC support is a no-op now, so there's > no functional change in this patch. > > If loaded manually, the legacy eeprom driver exposed the DDC EEPROM > to userspace. If this functionality is needed, then now the DDC > EEPROM has to be explicitly instantiated using at24. > > See also: > https://docs.kernel.org/i2c/instantiating-devices.html I'll take your word for it. Though none of the documentation I can find say that setting the class is legacy or deprecated or should be avoided. *shrug*. Acked-by: Jani Nikula <jani.nikula@intel.com> > > >> What does this mean? >> >> >> BR, >> Jani. >> > Heiner > >> >>> >>> Preferably this series should be applied via the i2c tree. >>> >>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >>> >>> --- >>> drivers/gpu/drm/i915/display/intel_gmbus.c | 1 - >>> drivers/gpu/drm/i915/display/intel_sdvo.c | 1 - >>> 2 files changed, 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c >>> index 40d7b6f3f..e9e4dcf34 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c >>> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c >>> @@ -899,7 +899,6 @@ int intel_gmbus_setup(struct drm_i915_private *i915) >>> } >>> >>> bus->adapter.owner = THIS_MODULE; >>> - bus->adapter.class = I2C_CLASS_DDC; >>> snprintf(bus->adapter.name, >>> sizeof(bus->adapter.name), >>> "i915 gmbus %s", gmbus_pin->name); >>> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c >>> index a636f42ce..5e64d1baf 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c >>> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c >>> @@ -3311,7 +3311,6 @@ intel_sdvo_init_ddc_proxy(struct intel_sdvo_ddc *ddc, >>> ddc->ddc_bus = ddc_bus; >>> >>> ddc->ddc.owner = THIS_MODULE; >>> - ddc->ddc.class = I2C_CLASS_DDC; >>> snprintf(ddc->ddc.name, I2C_NAME_SIZE, "SDVO %c DDC%d", >>> port_name(sdvo->base.port), ddc_bus); >>> ddc->ddc.dev.parent = &pdev->dev; >>> >> >
On 13.11.2023 18:50, Jani Nikula wrote: > On Mon, 13 Nov 2023, Heiner Kallweit <hkallweit1@gmail.com> wrote: >> On 13.11.2023 13:17, Jani Nikula wrote: >>> On Mon, 13 Nov 2023, Heiner Kallweit <hkallweit1@gmail.com> wrote: >>>> After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in >>>> olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. >>>> Class-based device auto-detection is a legacy mechanism and shouldn't >>>> be used in new code. So we can remove this class completely now. >>> >>> So this is copy-pasted to all commits and the cover letter, but please >>> do explain why there are no functional changes here (or are there?), >>> without me having to go through the i2c stack and try to find the >>> commits alluded to in "After removal of the legacy ...". >>> >> Legacy eeprom driver was marked deprecated 4 yrs ago with: >> 3079b54aa9a0 ("eeprom: Warn that the driver is deprecated") >> Now it has been removed with: >> 0113a99b8a75 ("eeprom: Remove deprecated legacy eeprom driver") >> >> Declaration of I2C_CLASS_DDC support is a no-op now, so there's >> no functional change in this patch. >> >> If loaded manually, the legacy eeprom driver exposed the DDC EEPROM >> to userspace. If this functionality is needed, then now the DDC >> EEPROM has to be explicitly instantiated using at24. >> >> See also: >> https://docs.kernel.org/i2c/instantiating-devices.html > > I'll take your word for it. Though none of the documentation I can find > say that setting the class is legacy or deprecated or should be > avoided. *shrug*. > I have to agree that it's not obvious that class-based instantiation is considered a legacy mechanism. The commit message of this 9 yrs old commit provides an explanation. 0c176170089c ("i2c: add deprecation warning for class based instantiation") > Acked-by: Jani Nikula <jani.nikula@intel.com> > Thanks > >> >> >>> What does this mean? >>> >>> >>> BR, >>> Jani. >>> >> Heiner >> >>> >>>> >>>> Preferably this series should be applied via the i2c tree. >>>> >>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >>>> >>>> --- >>>> drivers/gpu/drm/i915/display/intel_gmbus.c | 1 - >>>> drivers/gpu/drm/i915/display/intel_sdvo.c | 1 - >>>> 2 files changed, 2 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c >>>> index 40d7b6f3f..e9e4dcf34 100644 >>>> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c >>>> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c >>>> @@ -899,7 +899,6 @@ int intel_gmbus_setup(struct drm_i915_private *i915) >>>> } >>>> >>>> bus->adapter.owner = THIS_MODULE; >>>> - bus->adapter.class = I2C_CLASS_DDC; >>>> snprintf(bus->adapter.name, >>>> sizeof(bus->adapter.name), >>>> "i915 gmbus %s", gmbus_pin->name); >>>> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c >>>> index a636f42ce..5e64d1baf 100644 >>>> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c >>>> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c >>>> @@ -3311,7 +3311,6 @@ intel_sdvo_init_ddc_proxy(struct intel_sdvo_ddc *ddc, >>>> ddc->ddc_bus = ddc_bus; >>>> >>>> ddc->ddc.owner = THIS_MODULE; >>>> - ddc->ddc.class = I2C_CLASS_DDC; >>>> snprintf(ddc->ddc.name, I2C_NAME_SIZE, "SDVO %c DDC%d", >>>> port_name(sdvo->base.port), ddc_bus); >>>> ddc->ddc.dev.parent = &pdev->dev; >>>> >>> >> >
On 13.11.2023 18:49, Wolfram Sang wrote: > >> Preferably this series should be applied via the i2c tree. > > Are we in a hurry here, i.e. does it block further development of the > i801 smbus driver? My gut feeling says the patches should rather go via > drm and fbdev trees, but I may be convinced otherwise. > We're not in a hurry. It's just my experience with patch series' affecting multiple subsystems that typically the decision was to apply the full series via one tree. Also to avoid inquires from maintainers like: Shall I take it or are you going to take it? Of course there may be different opinions. Please advise.
> We're not in a hurry. It's just my experience with patch series' affecting > multiple subsystems that typically the decision was to apply the full > series via one tree. Also to avoid inquires from maintainers like: > Shall I take it or are you going to take it? > Of course there may be different opinions. Please advise. Ok, then this turns out to be a negotation thing between the drm/fbdev maintainers and me. I *can* take all the patches, of course. But since the number of patches touching the non-i2c subsystems is high, I'd like to hear their preference, too.
On Mon, Nov 13, 2023 at 3:55 PM Wolfram Sang <wsa@kernel.org> wrote: > > > > We're not in a hurry. It's just my experience with patch series' affecting > > multiple subsystems that typically the decision was to apply the full > > series via one tree. Also to avoid inquires from maintainers like: > > Shall I take it or are you going to take it? > > Of course there may be different opinions. Please advise. > > Ok, then this turns out to be a negotation thing between the drm/fbdev > maintainers and me. I *can* take all the patches, of course. But since > the number of patches touching the non-i2c subsystems is high, I'd like > to hear their preference, too. > I don't have a preference if you would prefer to take these via i2c. Alex
After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC. Class-based device auto-detection is a legacy mechanism and shouldn't be used in new code. So we can remove this class completely now. Preferably this series should be applied via the i2c tree. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_i2c.c | 1 - drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 1 - drivers/gpu/drm/ast/ast_i2c.c | 1 - drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 1 - drivers/gpu/drm/display/drm_dp_helper.c | 1 - drivers/gpu/drm/display/drm_dp_mst_topology.c | 1 - drivers/gpu/drm/gma500/cdv_intel_dp.c | 1 - drivers/gpu/drm/gma500/intel_gmbus.c | 1 - drivers/gpu/drm/gma500/oaktrail_hdmi_i2c.c | 1 - drivers/gpu/drm/gma500/psb_intel_sdvo.c | 1 - drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c | 1 - drivers/gpu/drm/i915/display/intel_gmbus.c | 1 - drivers/gpu/drm/i915/display/intel_sdvo.c | 1 - drivers/gpu/drm/loongson/lsdc_i2c.c | 1 - drivers/gpu/drm/mediatek/mtk_hdmi_ddc.c | 1 - drivers/gpu/drm/mgag200/mgag200_i2c.c | 1 - drivers/gpu/drm/msm/hdmi/hdmi_i2c.c | 1 - drivers/gpu/drm/radeon/radeon_i2c.c | 1 - drivers/gpu/drm/rockchip/inno_hdmi.c | 1 - drivers/gpu/drm/rockchip/rk3066_hdmi.c | 1 - drivers/gpu/drm/sun4i/sun4i_hdmi_i2c.c | 1 - drivers/video/fbdev/core/fb_ddc.c | 1 - drivers/video/fbdev/cyber2000fb.c | 1 - drivers/video/fbdev/i740fb.c | 1 - drivers/video/fbdev/intelfb/intelfb_i2c.c | 15 +++++---------- drivers/video/fbdev/matrox/i2c-matroxfb.c | 12 ++++-------- drivers/video/fbdev/s3fb.c | 1 - drivers/video/fbdev/tdfxfb.c | 1 - drivers/video/fbdev/tridentfb.c | 1 - drivers/video/fbdev/via/via_i2c.c | 1 - include/linux/i2c.h | 1 - 31 files changed, 9 insertions(+), 47 deletions(-)