Moi, On Tue, Oct 10, 2023 at 09:52:48AM +0300, Tomi Valkeinen wrote: > On 10/10/2023 01:09, Sakari Ailus wrote: > > Hi folks, > > > > This set includes a helper for working with V4L2 sub-device active state as > > well as a fix for the CCS driver sub-device state patch. > > > > since v1: > > > > - There were other drivers using __v4l2_subdev_state_alloc(). Ouch. Change > > those users as well. Note that this function should not be used in > > drivers, hence API niceness is not a high priority. > > > > Sakari Ailus (4): > > media: v4l: subdev: Set sub-device active state earlier > > media: v4l: subdev: Add a helper to tell if a sub-device state is > > active > > media: ccs: Rework initialising sub-device state > > media: ccs: Fix a (harmless) lockdep warning > > > > drivers/media/i2c/ccs/ccs-core.c | 64 ++++++++++++------- > > .../platform/renesas/rcar-vin/rcar-v4l2.c | 3 +- > > .../media/platform/renesas/vsp1/vsp1_entity.c | 3 +- > > drivers/media/v4l2-core/v4l2-subdev.c | 14 ++-- > > drivers/staging/media/tegra-video/vi.c | 2 +- > > include/media/v4l2-subdev.h | 11 +++- > > 6 files changed, 66 insertions(+), 31 deletions(-) > > > > I'm not familiar with the CCS driver, and you don't explain much why you are > doing this, so it's a bit unclear to me. But I don't like it. > > The idea with the subdev state was that the driver doesn't need to know > whether it's active-state or try-state. It should behave the same way in > both cases. This series goes against that. > > Can you explain a bit what the issue is and what you are doing in this > series? The driver maintains internal state and that needs to be updated when the configuration (including what's in sub-device state) changes. Generally the driver knows (as for the whence field) which state it's dealing with but that is not the case for init_cfg. Alternatively I could split the internal workings of the driver into active and try states but I prefer to improve the framework and make the driver simpler. Deducing the internal configuration solely based on sub-device state is not really feasible. -- Terveisin, Sakari Ailus
