| Message ID | 20250507184737.154747-1-hdegoede@redhat.com |
|---|---|
| Headers | show |
| Series | platform/x86: int3472: Allow re-using sensor GPIO mapping in atomisp | expand |
On Thu, May 8, 2025 at 11:36 AM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Wed, May 7, 2025 at 9:52 PM Hans de Goede <hdegoede@redhat.com> wrote: > > > This patch series does some small refactoring of the int3472 code to allow > > re-using the sensor GPIO mapping code in the atomisp driver and then the > > final patch in the series moves the atomisp driver over. > > > > About merging this, maybe the int3472 patches can be merged in time for > > the 6.16 merge window and then the atomisp patch can be merged after > > 6.16-rc1 is released, otherwise an immutable pdx86 branch with the first > > 5 patches will be necessary. > > Overall I'm pretty much liking this series, but one comment against > the last patch (see there) and one question here. Can you isolate GPIO > mapping code in a separate file, please? This will help to generalise > this code outside of two mentioned drivers (I might need it in the > future for something else, not related to cameras at all). Ah, and formal Reviewed-by: Andy Shevchenko <andy@kernel.org> for patches 1-5.
Hi Andy, On 8-May-25 10:36 AM, Andy Shevchenko wrote: > On Wed, May 7, 2025 at 9:52 PM Hans de Goede <hdegoede@redhat.com> wrote: > >> This patch series does some small refactoring of the int3472 code to allow >> re-using the sensor GPIO mapping code in the atomisp driver and then the >> final patch in the series moves the atomisp driver over. >> >> About merging this, maybe the int3472 patches can be merged in time for >> the 6.16 merge window and then the atomisp patch can be merged after >> 6.16-rc1 is released, otherwise an immutable pdx86 branch with the first >> 5 patches will be necessary. > > Overall I'm pretty much liking this series, but one comment against > the last patch (see there) and one question here. Can you isolate GPIO > mapping code in a separate file, please? This will help to generalise > this code outside of two mentioned drivers (I might need it in the > future for something else, not related to cameras at all). If you want to re-use this elsewhere then splitting it out further sounds like a good plan. But which bits do you need? Do you actually need the full code calling the special DSM and then adding either GPIO-lookups, or gpio controlled regulators / clks / LEDs ? Because atm the int3472/discrete.c + other c files linked into the .ko does all of that and for the atomisp2 case we actually want all of that (although for now GPIO -> clk and LED is unused there). Anyway I think it would be best for you (Andy) to come up with a proposal / RFC patch series to split out what you need. I'm certainly open to that and happy to review such a series. Regards, Hans
On Thu, May 8, 2025 at 5:00 PM Hans de Goede <hdegoede@redhat.com> wrote: > On 8-May-25 10:36 AM, Andy Shevchenko wrote: > > On Wed, May 7, 2025 at 9:52 PM Hans de Goede <hdegoede@redhat.com> wrote: ... > > Can you isolate GPIO > > mapping code in a separate file, please? This will help to generalise > > this code outside of two mentioned drivers (I might need it in the > > future for something else, not related to cameras at all). > > If you want to re-use this elsewhere then splitting it out > further sounds like a good plan. > > But which bits do you need? Do you actually need the full code calling > the special DSM and then adding either GPIO-lookups, or gpio controlled > regulators / clks / LEDs ? > > Because atm the int3472/discrete.c + other c files linked into the .ko > does all of that and for the atomisp2 case we actually want all of > that (although for now GPIO -> clk and LED is unused there). I basically want to have remap\ing quirk part to be isolated. > Anyway I think it would be best for you (Andy) to come up with > a proposal / RFC patch series to split out what you need. I'm certainly > open to that and happy to review such a series. Fine, but can you do this in the discrete.c internally, like an embedded C file so it doesn't require headers to be used, but being an interface-ready solution?
Hi All, This patch series does some small refactoring of the int3472 code to allow re-using the sensor GPIO mapping code in the atomisp driver and then the final patch in the series moves the atomisp driver over. About merging this, maybe the int3472 patches can be merged in time for the 6.16 merge window and then the atomisp patch can be merged after 6.16-rc1 is released, otherwise an immutable pdx86 branch with the first 5 patches will be necessary. Regards, Hans Hans de Goede (6): platform/x86: int3472: Move common.h to public includes, symbols to INTEL_INT3472 platform/x86: int3472: Stop using devm_gpiod_get() platform/x86: int3472: Export int3472_discrete_parse_crs() platform/x86: int3472: Remove unused sensor_config struct member platform/x86: int3472: For mt9m114 sensors map powerdown to powerenable media: atomisp: Switch to int3472 driver sensor GPIO mapping code MAINTAINERS | 1 + .../x86/intel/int3472/clk_and_regulator.c | 9 +- drivers/platform/x86/intel/int3472/common.c | 9 +- drivers/platform/x86/intel/int3472/discrete.c | 28 +- .../x86/intel/int3472/discrete_quirks.c | 3 +- drivers/platform/x86/intel/int3472/led.c | 3 +- drivers/platform/x86/intel/int3472/tps68470.c | 3 +- .../staging/media/atomisp/pci/atomisp_csi2.h | 17 -- .../media/atomisp/pci/atomisp_csi2_bridge.c | 239 ++---------------- .../staging/media/atomisp/pci/atomisp_v4l2.c | 1 + .../linux/platform_data/x86/int3472.h | 16 +- 11 files changed, 76 insertions(+), 253 deletions(-) rename drivers/platform/x86/intel/int3472/common.h => include/linux/platform_data/x86/int3472.h (92%)