Re: [PATCH 2/2] dw9807-vcm: Add "dongwoon,dw9807" compatible string

[Sakari Ailus <sakari.ailus@linux.intel.com>]

On Wed, Mar 23, 2022 at 03:59:03PM +0100, Krzysztof Kozlowski wrote:
> On 23/03/2022 15:50, Sakari Ailus wrote:
> > Hi Krysztof,
> > 
> > On Sun, Mar 20, 2022 at 12:58:08PM +0100, Krzysztof Kozlowski wrote:
> >> On 18/03/2022 17:51, Sakari Ailus wrote:
> >>> There is firmware out there that uses "dongwoon,dw9807" compatible string
> >>> that never made it to upstream as-is. Add it to the driver to make it load
> >>> on such systems.
> >>>
> >>> The chip also has an EEPROM part which is AT24 compatible (for reading
> >>> purposes) on a separate I²C address. Adding possible support for this in
> >>> the future is not affected by this change.
> >>>
> >>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> >>> ---
> >>>  drivers/media/i2c/dw9807-vcm.c | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/drivers/media/i2c/dw9807-vcm.c b/drivers/media/i2c/dw9807-vcm.c
> >>> index 95e06f13bc9ed..ada8e467a0450 100644
> >>> --- a/drivers/media/i2c/dw9807-vcm.c
> >>> +++ b/drivers/media/i2c/dw9807-vcm.c
> >>> @@ -295,6 +295,8 @@ static int  __maybe_unused dw9807_vcm_resume(struct device *dev)
> >>>  
> >>>  static const struct of_device_id dw9807_of_table[] = {
> >>>  	{ .compatible = "dongwoon,dw9807-vcm" },
> >>> +	/* Compatibility for older firmware */
> >>> +	{ .compatible = "dongwoon,dw9807" },
> >>
> >> You need to add it to the bindings as well.
> > 
> > Why? Generally things that are there for binary compatibility but
> > deprecated are not documented in bindings.
> 
> Because:
> 1. Otherwise it will get removed by someone as undocumented, unknown
> stuff. Someone might treat it as mistake.
> 2. Putting it in the driver code is already self-documenting, so someone
> might start using it. This should be clearly discouraged and bindings
> help in it - the compatible is marked as deprecated with the comment you
> mentioned.

That's why there's a comment. I could add a warning telling not to use it
anywhere.

This really does not belong to bindings.

> 3. Bindings are used beyond Linux kernel, so they might document
> something more than we use here in DT.

-- 
Sakari Ailus