Re: [PATCH v6 3/4] platform: int3472: discrete: con_id vana for Sony IMX471 as power enable

[Tarang Raval]

Hi Hans, Sakari

> On Wed, Jul 01, 2026 at 01:01:58PM +0200, Hans de Goede wrote:
> > Hi,
> >
> > On 1-Jul-26 08:19, Tarang Raval wrote:
> > > Hi Hans,
> > >
> > >> On 30-Jun-26 09:32, Tarang Raval wrote:
> > >>> Hi Kate,
> > >>>
> > >>>> Update the con_id for the Sony IMX471 sensor to "vana" to serve as the
> > >>>> power enable. Additionally, the HID values SONY471A and TBE20A0, both
> > >>>> associated with the IMX471 image sensor, have been identified on Lenovo
> > >>>> laptops.
> > >>>>
> > >>>> Signed-off-by: Kate Hsuan <hpa@xxxxxxxxxx>
> > >>>
> > >>> Thanks, looks good.
> > >>>
> > >>> Reviewed-by: Tarang Raval <tarang.raval@xxxxxxxxxxxxxxxxx>
> > >>
> > >> Hmm, the imx471 driver is still pending upstream:
> > >>
> > >> https://lore.kernel.org/linux-media/20260629074026.35490-5-hpa@xxxxxxxxxx/
> > >>
> > >> As part of this series.
> > >>
> > >> Please just use the standardized "avdd" in that driver instead
> > >> of "vana" (which also seems to refer to the analog supply vdd,
> > >> which is what avdd stands for).
> > >>
> > >> Then this whole patch is unnecessary and can be dropped from
> > >> this series.
> > >
> > > The regulator name "vana" comes directly from the Sony IMX471 sensor
> > > datasheet, which typically refers to the analog supply voltage. Using the
> > > datasheet name helps keep the driver consistent with the hardware
> > > documentation and makes it easier to cross-reference.
> > >
> > > as per my understanding, the more standardized way is to use the regulator
> > > name as per the sensor datasheet. Therefore, I respectfully disagree with
> > > your suggestion.
> >
> > As shown by the need for this patch on x86 at least because there
> > is no devicetree it greatly helps if all Linux sensor drivers use
> > standardized names for their regulators rather then using the exact name
> > from the datasheet which often is not very consistent.
> >
> > And "avdd" is the name we've standardized on for this, so lets use that:
> >
> > hans@shalem:~/projects/linux$ grep -l '"vana"' drivers/media/i2c/*.c | wc -l
> > 4
> > hans@shalem:~/projects/linux$ grep -l '"avdd"' drivers/media/i2c/*.c | wc -l
> > 36
> >
> > The alternative is needing to add more and more quirks as different
> > sensors are used, which is not great.
>
> I do agree that having a constant name for the regulators would be
> beneficial for the int3472 driver. Still, if, and presumably, when that
> sensor gets DT support, the bindings will use the regulator name from the
> datasheet.
>
> Let's just use the datasheet name now and add the few lines needed to the
> int3472 driver and avoid the churn in the future. There's a limited number
> of sensor drivers that need this after all.
>
> I'd be more concerned of what's going on in tps68470_board_data.c for
> instance.

I went through the INT3472 driver and would like to propose a generic     
approach that satisfies both sides without per-HID quirks or sensor driver
changes.                                                                  
                                                                          
The problem is:                                                      
 - INT3472 standardizes on "avdd" internally                             
 - Sony IMX sensor drivers use "vana" per datasheet, and all existing    
   Sony DT bindings (imx219, imx290, imx415) already use vana-supply     
 - Changing imx471 to "avdd" now will create inconsistency with those    
   bindings, or require a rename later                                   
                                                                          
Instead of fixing this per-sensor (either by changing the driver or adding
a per-HID entry to int3472_gpio_map), we can add a small vendor alias     
table inside skl_int3472_register_regulator().                            
                                                                          
Currently that function registers two consumer supply entries per sensor: 
lowercase ("avdd") and uppercase ("AVDD"). We can extend it to also       
register vendor datasheet aliases from a static table, e.g.:              
                                                                          
  avdd -> vana   (Sony IMX series: imx219, imx290, imx415, imx471, ...)   
                                                                          
This way, when a sensor driver requests "vana", the regulator framework   
finds it in the supply map without any extra quirks.                      

Best Regards,
Tarang