Re: [alsa-devel] [PATCH] ASoC: fsl_ssi: Remove unneeded 'i2s-slave' property
Sorry, I missed to add alsa-devel@alsa-project.org on Cc. ________________________________________ From: Timur Tabi timur@tabi.org Sent: Thursday, September 18, 2014 3:37 PM To: Nicolin Chen Cc: Estevam Fabio-R49496; broonie@kernel.org; Xiubo Li-B47053 Subject: Re: [PATCH] ASoC: fsl_ssi: Remove unneeded 'i2s-slave' property
On 09/18/2014 01:32 PM, Nicolin Chen wrote:
On Thu, Sep 18, 2014 at 03:11:17PM -0300, Fabio Estevam wrote:
There is no need to use 'i2s-slave' property, since master/slave configuration are passed via machine layer.
This change does not break existing users because they do check for slave mode inside sound/soc/fsl/mpc8610_hpcd.c/p1022_ds.c and p1022_rdk.c
Besides this change, I'm wondering if we can take a further step to drop this entire property from Device Tree binding document as newer platforms may no long need this property in their DT bindings.
I think the property is necessary if anyone takes a P1022 or MPC8610 and uses it in a platform that doesn't support i2s-slave. But currently, the code doesn't support anything else. So technically, we could remove it, and if anyone does want to support another configuration, he will have to add that support the "new" way.
On Thu, Sep 18, 2014 at 06:46:42PM +0000, Fabio.Estevam@freescale.com wrote:
On 09/18/2014 01:32 PM, Nicolin Chen wrote:
On Thu, Sep 18, 2014 at 03:11:17PM -0300, Fabio Estevam wrote:
There is no need to use 'i2s-slave' property, since master/slave configuration are passed via machine layer.
This change does not break existing users because they do check for slave mode inside sound/soc/fsl/mpc8610_hpcd.c/p1022_ds.c and p1022_rdk.c
Besides this change, I'm wondering if we can take a further step to drop this entire property from Device Tree binding document as newer platforms may no long need this property in their DT bindings.
I think the property is necessary if anyone takes a P1022 or MPC8610 and uses it in a platform that doesn't support i2s-slave. But currently, the code doesn't support anything else. So technically, we could remove it, and if anyone does want to support another configuration, he will have to add that support the "new" way.
Thank you for the comments. Then we may use compatible check for P1022 and MPC8610 to limit their DAI formats.
For the property, do you agree to modify it like this:
- fsl,mode: The operating mode for the AC97 interface only. "ac97-slave" - AC97 mode, SSI is clock slave "ac97-master" - AC97 mode, SSI is clock master (There's no need for other DAI format like I2S mode to engage this property.)
And how about this patch? I'm personally okay with this change. We may also send an extra patch to refine the document after the agreement's done.
Thanks Nicolin
Nicolin Chen wrote:
Thank you for the comments. Then we may use compatible check for P1022 and MPC8610 to limit their DAI formats.
For the property, do you agree to modify it like this:
- fsl,mode: The operating mode for the AC97 interface only. "ac97-slave" - AC97 mode, SSI is clock slave "ac97-master" - AC97 mode, SSI is clock master (There's no need for other DAI format like I2S mode to engage this property.)
And how about this patch? I'm personally okay with this change. We may also send an extra patch to refine the document after the agreement's done.
You can't just redefine a property, because that breaks backwards compatibility with older device trees or older kernels.
On Thu, Sep 18, 2014 at 07:29:32PM -0500, Timur Tabi wrote:
For the property, do you agree to modify it like this:
- fsl,mode: The operating mode for the AC97 interface only. "ac97-slave" - AC97 mode, SSI is clock slave "ac97-master" - AC97 mode, SSI is clock master (There's no need for other DAI format like I2S mode to engage this property.)
You can't just redefine a property, because that breaks backwards compatibility with older device trees or older kernels.
Sorry, I'm not sure what's the problem here. It just drops i2s stuff and leave AC97 part as we discussed. Why would old DTs be broken? Is that possible for you to give me an example to understand the point?
Thank you Nicolin
Nicolin Chen wrote:
You can't just redefine a property, because that breaks backwards
compatibility with older device trees or older kernels.
Sorry, I'm not sure what's the problem here. It just drops i2s stuff and leave AC97 part as we discussed. Why would old DTs be broken? Is that possible for you to give me an example to understand the point?
If you have an older kernel with a newer device tree (that doesn't have "i2s-mode"), the driver will fail to load because "i2s-mode" is missing.
On Fri, Sep 19, 2014 at 07:51:46AM -0500, Timur Tabi wrote:
Nicolin Chen wrote:
You can't just redefine a property, because that breaks backwards
compatibility with older device trees or older kernels.
Sorry, I'm not sure what's the problem here. It just drops i2s stuff and leave AC97 part as we discussed. Why would old DTs be broken? Is that possible for you to give me an example to understand the point?
If you have an older kernel with a newer device tree (that doesn't have "i2s-mode"), the driver will fail to load because "i2s-mode" is missing.
I thought only the backward compatibility for DT is demanded..... But I still don't get why people would use new DT without updating kernel. It's pretty fair to use old DT with newer kernel because they want bug-fix patches.
Thank you Nicolin
Nicolin Chen wrote:
I thought only the backward compatibility for DT is demanded.....
I have higher standards :-)
But I still don't get why people would use new DT without updating kernel. It's pretty fair to use old DT with newer kernel because they want bug-fix patches.
The device tree is shipped with the kernel, so some people say that backwards compatibility is not needed anywhere. I think it's important to provide backwards compatibility both ways.
I generally prefer some kind of transition period, where you can use new or old kernel with a new or old device tree.
However, let me look at Fabio's patches some more.
On Fri, Sep 19, 2014 at 06:49:18PM -0500, Timur Tabi wrote:
Nicolin Chen wrote:
I thought only the backward compatibility for DT is demanded.....
I have higher standards :-)
But I still don't get why people would use new DT without updating kernel. It's pretty fair to use old DT with newer kernel because they want bug-fix patches.
The device tree is shipped with the kernel, so some people say that backwards compatibility is not needed anywhere. I think it's important to provide backwards compatibility both ways.
I generally prefer some kind of transition period, where you can use new or old kernel with a new or old device tree.
I see..
However, let me look at Fabio's patches some more.
Yes, please.
participants (3)
-
Fabio.Estevam＠freescale.com -
Nicolin Chen -
Timur Tabi