Hi,

On Tue, Dec 08, 2015 at 11:51:44AM +0100, Danny Milosavljevic wrote:
> Hi Maxime,
> 
> On Tue, 8 Dec 2015 10:09:50 +0100
> Maxime Ripard <[email protected]> wrote:
> 
> > > Now I added an ugly hack to make Mic Gain work on A10 as well - but I 
> > > don't have 
> > > A10 hardware. Let's see what they say.
> > 
> > That I really prefer having nothing over an "ugly hack" :)
> 
> I figured you'd say that :)
> 
> But for these kind of architectural decisions should I send the question to 
> all 
> the people get_maintainers tells me in advance? Or pick one?

All of them.

> Or should I include it in the patch E-Mail above the "---"?

You can do that, but usually there's no strong reason to do this for a
single patch, you can just put it on the git send-email command line.

> Like this:
> 
> PREG1 is the Mic1-In Pre-Amplifier gain control.
> PREG2 is the Mic2-In Pre-Amplifier gain control.
> 
> The A10 has PREG1 and PREG2 in the ADC control register.
> The A20 has PREG1 and PREG2 in the (new) PHONE_CAL control register.
> 
> Right now my (v5) probe function modifies the global variable 
> "sun4i_codec_widgets" depending on the compatible string.
> I think it's safe for the time being since the codec is on-die and it's not 
> possible to suddenly need both sun4i widgets and sun7i widgets in the same 
> kernel at the same time.
> (I don't have A10 hardware. So I wrote it in a way that in the A10 case 
> it doesn't have to do anything and in the A20 case it does the modifications.
> That way, I could test the modifications (I do have A20 hardware))

Yeah, that's not so nice.

> The alternatives would be to either
> 1) split the driver up into kernel modules "sun4i-codec" and "sun7i-codec". 
>    Maybe add a common object file to reduce duplicate work.
>    This would be an user-visible change. Is that even allowed?

Not really either, if the hardware as similar, you just have the same
driver for the two.

> 2) make the "sun4i-codec" kernel module register two different codecs 
>    with two different compatibles and two different "sun4i"_codec_widget 
>    variables. Is that even possible?

Yes, you have a data field you can associate to the compatible in the
of_device_id structure, and that you can later retrieve using
of_device_get_match_data.

> 3) make ALSA dynamically add two different widgets at run time depending on 
>    the compatible (not added to the "sun4i_codec_widget" array). 
>    Is that even possible?

There's no need to have two at the same time, they're mutually
exclusive.

4) Copy the structure in probe and modify the copied instance

I guess 2 or 4 are the two valid way of doing things.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Attachment: signature.asc
Description: Digital signature

Reply via email to