[PATCH] ASoC: fsl_ssi: Remove unneeded 'i2s-slave' property

[F***@freescale.com]

Sorry, I missed to add alsa-***@alsa-project.org on Cc.
________________________________________
From: Timur Tabi <***@tabi.org>
Sent: Thursday, September 18, 2014 3:37 PM
To: Nicolin Chen
Cc: Estevam Fabio-R49496; ***@kernel.org; Xiubo Li-B47053
Subject: Re: [PATCH] ASoC: fsl_ssi: Remove unneeded 'i2s-slave' property

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.

[Nicolin Chen]

Post by F***@freescale.com

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

[Timur Tabi]

Post by Nicolin Chen
Thank you for the comments. Then we may use compatible check for
P1022 and MPC8610 to limit their DAI formats.
- 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.

[Nicolin Chen]

Post by Timur Tabi

Post by Nicolin Chen
- 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

[Timur Tabi]

Post by Nicolin Chen

Post by Timur Tabi
You can't just redefine a property, because that breaks backwards

Post by Timur Tabi
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.

[Nicolin Chen]

Post by Timur Tabi

Post by Nicolin Chen

Post by Timur Tabi
You can't just redefine a property, because that breaks backwards

Post by Timur Tabi
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

[Timur Tabi]

Post by Nicolin Chen
I thought only the backward compatibility for DT is demanded.....

I have higher standards :-)

Post by Nicolin Chen
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.

[Nicolin Chen]

Post by Timur Tabi

Post by Nicolin Chen
I thought only the backward compatibility for DT is demanded.....

I have higher standards :-)

Post by Nicolin Chen
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..

Post by Timur Tabi
However, let me look at Fabio's patches some more.

Yes, please.