Re: [PATCH v3 2/2] drm/bridge: This patch add new DRM bridge driver for LT9611C(EX/UXD) chip

[杨孙运]

Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx> 于2026年4月23日周四 02:56写道：
>
> On Wed, Apr 22, 2026 at 12:41:11PM +0800, 杨孙运 wrote:
> > Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx> 于2026年4月21日周二 18:48写道：
> > >
> > > On Tue, Apr 21, 2026 at 03:37:52PM +0800, 杨孙运 wrote:
> > > > Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx> 于2026年4月20日周一 20:18写道：
> > > > >
> > > > > On Mon, Apr 20, 2026 at 02:16:44PM +0800, syyang@xxxxxxxxxxx wrote:
> > > > > > From: Sunyun Yang <syyang@xxxxxxxxxxx>
> > > >
> > > > > > +
> > > > > > +static int lt9611c_hdmi_audio_prepare(struct drm_bridge *bridge,
> > > > > > +                                   struct drm_connector *connector,
> > > > > > +                                   struct hdmi_codec_daifmt *fmt,
> > > > > > +                                   struct hdmi_codec_params *hparms)
> > > > > > +{
> > > > > > +     struct lt9611c *lt9611c = bridge_to_lt9611c(bridge);
> > > > > > +     u8 audio_cmd[6] = {0x57, 0x48, 0x36, 0x3a};
> > > > > > +     u8 data[5];
> > > > > > +     int ret;
> > > > > > +
> > > > > > +     /* Validate sample rate and width (LT9611C auto-detects but we still check) */
> > > > >
> > > > > What for? You don't trust ASoC / ALSA core that the rates would match
> > > > > HDMI_RATES?
> > > >
> > > > Chip has limitations on sample_rate, sample_width, and fmt, and cannot
> > > > support all formats.
> > > > The validation here is to ensure that only the formats supported by
> > > > the chip are used.
> > >
> > > Again, for sample rates, is it different from HDMI_RATES? If not, it is
> > > useless protective coding.
> > >
> >
> > I checked the definition of HDMI_RATES and ended up doing a redundant
> > validation.
> >
> > #define HDMI_RATES (SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 |\
> > SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 |\
> > SNDRV_PCM_RATE_96000 | SNDRV_PCM_RATE_176400 |\
> > SNDRV_PCM_RATE_192000)
> >
> > However, if HDMI_RATES is extended in the future to include sample
> > rate not supported by LT9611C, this driver would require corresponding
> > updates.
> > If this is not expected to happen, I can remove the redundant check.
>
> Updating HDMI_RATES would cause enough issues for the existign drivers
> which don't have this kind of a check.
>
OK,
it will be removed in next version.

> >
> > > > >
> > > > > > +     switch (hparms->sample_rate) {
> > > > > > +     case 32000:
> > > > > > +     case 44100:
> > > > > > +     case 48000:
> > > > > > +     case 88200:
> > > > > > +     case 96000:
> > > > > > +     case 176400:
> > > > > > +     case 192000:
> > > > > > +             break;
> > > > > > +     default:
> > > > > > +             return -EINVAL;
> > > > > > +     }
> > > > > > +
> > > > > > +     switch (hparms->sample_width) {
> > > > > > +     case 16:
> > > > > > +     case 18:
> > > > > > +     case 20:
> > > > > > +     case 24:
> > > > >
> > > > > and no support for 32?
> > > > >
> > > > no support for 32
> > >
> > > Then check for that rather than listing all the widths. If you check the
> > > hdmi-codec, you can't get 18-bit samples at all.
> > >
> > You are right, there is no 18-bit support.
> >
> > same reason with sample rate.
>
> Yep. Check for 32-bit, the rest is fine.
>
Same , it will be fixed in next version.

> >
> > > > > > +MODULE_LICENSE("GPL v2");
> > > > >
> > > > >
> > > > > I think, checkpatch.pl should have flagged this.
> > > > >
> > > > yes,  it is need use GPL?
> > >
> > > If it flagged the line, why didn't you fix it? What did checkpatch say?
> > >
> > Sorry, it is my mistake. i will fix.
> > checkpatch say:
> > WARNING: Prefer "GPL" over "GPL v2" - see commit bf7fbeeae6db
> > ("module: Cure the MODULE_LICENSE "GPL" vs. "GPL v2" bogosity")
> > #1450: FILE: drivers/gpu/drm/bridge/lontium-lt9611c.c:1364:
> >
>
> So... If there was a warning, why did you ignore it? Please don't,
> unless you really know what you are doing.
>

it will use GPL in next version.

> >
> > > > I see that many of the latest code still use GPL v2.
> > >
> > > --
> > > With best wishes
> > > Dmitry
>
> --
> With best wishes
> Dmitry