Re: [PATCH 08/13] drm/i2c: tda998x: move audio routing configuration

[Russell King - ARM Linux admin <linux@armlinux.org.uk>]

On Wed, Jun 12, 2019 at 11:36:59AM -0400, Sven Van Asbroeck wrote:
> On Tue, Jun 11, 2019 at 7:02 AM Russell King <rmk+kernel@armlinux.org.uk> wrote:
> >
> > Move the mux and clocking selection out of tda998x_configure_audio()
> > into the parent functions, so we can validate this when parameters
> > are set outside of the audio mutex.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> 
> +/* Configure the TDA998x audio data and clock routing. */
> +static int tda998x_derive_routing(struct tda998x_priv *priv,
> +                                 struct tda998x_audio_settings *s,
> +                                 unsigned int route)
> +{
> +       s->route = &tda998x_audio_route[route];
> +       s->ena_ap = priv->audio_port_enable[route];
> +       if (s->ena_ap == 0) {
> +               dev_err(&priv->hdmi->dev, "no audio configuration found\n");
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> 
> Nit: priv is nearly unused in this function.
> Maybe delegate the error log to the caller, in that case we could just pass
> route and const audio_port_enable to the function. Instead of passing in the
> 'kitchen sink' priv ?

I don't think that's worth doing.  This way, compilers are free to
emit code to bounds-check the audio_port_enable access since they
know that it is a defined size.  Passing in a const pointer ca
mean that check has to be avoided.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel