All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicolin Chen <nicoleotsuka@gmail.com>
To: "S.j. Wang" <shengjiu.wang@nxp.com>
Cc: Daniel Baluta <daniel.baluta@nxp.com>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"mark.rutland@arm.com" <mark.rutland@arm.com>,
	"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>,
	"festevam@gmail.com" <festevam@gmail.com>,
	dl-linux-imx <linux-imx@nxp.com>,
	Aisheng Dong <aisheng.dong@nxp.com>,
	Anson Huang <anson.huang@nxp.com>, Peng Fan <peng.fan@nxp.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ARM: dts: imx: Add mclk0 clock for SAI
Date: Sun, 21 Apr 2019 21:25:20 -0700	[thread overview]
Message-ID: <20190422042519.GA4304@Asurada> (raw)
In-Reply-To: <VE1PR04MB647951B20E0F334FF1CC3B7AE3220@VE1PR04MB6479.eurprd04.prod.outlook.com>

On Mon, Apr 22, 2019 at 03:30:26AM +0000, S.j. Wang wrote:
> > > SAI has 4 clock sources, which can be selected using MSEL bit of SAI
> > > TCR2 register.
> > 
> > I have a doubt at this statement. As far as I can understand, this MSEL is
> > probably used by its internal clock MUX, so it's not really proving that SAI
> > has 4 MCLK inputs. What I know is that SAI block itself only has 3 MCLK
> > inputs as we defined in DT. It's just internally connects bus clock or MCLK1
> > to input0 of clock MUX's and connects MCLK[1-3] to input[1-3]. So adding an
> > MCLK0 here doesn't sound a right way to me. Unless someone can justify
> > for it, I think we should just fix it from driver side.
> > 
> > Thanks
> > Nicolin
> >
> 
> The MSEL bit width is 2 bit, so there is 4 options,  the MCLK0 maybe the same input as
> MCLK1 or bus clock as you said, so we think may be better to show this relation in DT, 
> And this is DT's capability.  Driver don't care about which clock connect to MCLK0, 
> it only need to know there is 4 MCLK from DT. 

I know what it is. But it feels weird that we add an MCLK0 just
because of what a register filed has, and there's no "MCLK0" be
mentioned in the RM at all. My point is that if SAI doesn't have
a port named "MCLK0", I don't feel it's that convincing to have
it in the DT.

Usually in DT we define the clock sources of an entire IP block
in audio use cases, not for an internal clock MUX. But taking a
step back, it might not be really wrong to do so, since the MUX
is a part of the hardware. If we redefine the MCLK[0-4] as "four
clock sources of SAI's clock MUX selecting a clock for bit clock
and frame clock providing" in the binding doc, I feel it'd make
a lot of sense.

WARNING: multiple messages have this Message-ID (diff)
From: Nicolin Chen <nicoleotsuka@gmail.com>
To: "S.j. Wang" <shengjiu.wang@nxp.com>
Cc: "mark.rutland@arm.com" <mark.rutland@arm.com>,
	Aisheng Dong <aisheng.dong@nxp.com>, Peng Fan <peng.fan@nxp.com>,
	"festevam@gmail.com" <festevam@gmail.com>,
	Anson Huang <anson.huang@nxp.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Daniel Baluta <daniel.baluta@nxp.com>,
	"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	dl-linux-imx <linux-imx@nxp.com>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] ARM: dts: imx: Add mclk0 clock for SAI
Date: Sun, 21 Apr 2019 21:25:20 -0700	[thread overview]
Message-ID: <20190422042519.GA4304@Asurada> (raw)
In-Reply-To: <VE1PR04MB647951B20E0F334FF1CC3B7AE3220@VE1PR04MB6479.eurprd04.prod.outlook.com>

On Mon, Apr 22, 2019 at 03:30:26AM +0000, S.j. Wang wrote:
> > > SAI has 4 clock sources, which can be selected using MSEL bit of SAI
> > > TCR2 register.
> > 
> > I have a doubt at this statement. As far as I can understand, this MSEL is
> > probably used by its internal clock MUX, so it's not really proving that SAI
> > has 4 MCLK inputs. What I know is that SAI block itself only has 3 MCLK
> > inputs as we defined in DT. It's just internally connects bus clock or MCLK1
> > to input0 of clock MUX's and connects MCLK[1-3] to input[1-3]. So adding an
> > MCLK0 here doesn't sound a right way to me. Unless someone can justify
> > for it, I think we should just fix it from driver side.
> > 
> > Thanks
> > Nicolin
> >
> 
> The MSEL bit width is 2 bit, so there is 4 options,  the MCLK0 maybe the same input as
> MCLK1 or bus clock as you said, so we think may be better to show this relation in DT, 
> And this is DT's capability.  Driver don't care about which clock connect to MCLK0, 
> it only need to know there is 4 MCLK from DT. 

I know what it is. But it feels weird that we add an MCLK0 just
because of what a register filed has, and there's no "MCLK0" be
mentioned in the RM at all. My point is that if SAI doesn't have
a port named "MCLK0", I don't feel it's that convincing to have
it in the DT.

Usually in DT we define the clock sources of an entire IP block
in audio use cases, not for an internal clock MUX. But taking a
step back, it might not be really wrong to do so, since the MUX
is a part of the hardware. If we redefine the MCLK[0-4] as "four
clock sources of SAI's clock MUX selecting a clock for bit clock
and frame clock providing" in the binding doc, I feel it'd make
a lot of sense.

WARNING: multiple messages have this Message-ID (diff)
From: Nicolin Chen <nicoleotsuka@gmail.com>
To: "S.j. Wang" <shengjiu.wang@nxp.com>
Cc: "mark.rutland@arm.com" <mark.rutland@arm.com>,
	Aisheng Dong <aisheng.dong@nxp.com>, Peng Fan <peng.fan@nxp.com>,
	"festevam@gmail.com" <festevam@gmail.com>,
	Anson Huang <anson.huang@nxp.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Daniel Baluta <daniel.baluta@nxp.com>,
	"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	dl-linux-imx <linux-imx@nxp.com>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] ARM: dts: imx: Add mclk0 clock for SAI
Date: Sun, 21 Apr 2019 21:25:20 -0700	[thread overview]
Message-ID: <20190422042519.GA4304@Asurada> (raw)
In-Reply-To: <VE1PR04MB647951B20E0F334FF1CC3B7AE3220@VE1PR04MB6479.eurprd04.prod.outlook.com>

On Mon, Apr 22, 2019 at 03:30:26AM +0000, S.j. Wang wrote:
> > > SAI has 4 clock sources, which can be selected using MSEL bit of SAI
> > > TCR2 register.
> > 
> > I have a doubt at this statement. As far as I can understand, this MSEL is
> > probably used by its internal clock MUX, so it's not really proving that SAI
> > has 4 MCLK inputs. What I know is that SAI block itself only has 3 MCLK
> > inputs as we defined in DT. It's just internally connects bus clock or MCLK1
> > to input0 of clock MUX's and connects MCLK[1-3] to input[1-3]. So adding an
> > MCLK0 here doesn't sound a right way to me. Unless someone can justify
> > for it, I think we should just fix it from driver side.
> > 
> > Thanks
> > Nicolin
> >
> 
> The MSEL bit width is 2 bit, so there is 4 options,  the MCLK0 maybe the same input as
> MCLK1 or bus clock as you said, so we think may be better to show this relation in DT, 
> And this is DT's capability.  Driver don't care about which clock connect to MCLK0, 
> it only need to know there is 4 MCLK from DT. 

I know what it is. But it feels weird that we add an MCLK0 just
because of what a register filed has, and there's no "MCLK0" be
mentioned in the RM at all. My point is that if SAI doesn't have
a port named "MCLK0", I don't feel it's that convincing to have
it in the DT.

Usually in DT we define the clock sources of an entire IP block
in audio use cases, not for an internal clock MUX. But taking a
step back, it might not be really wrong to do so, since the MUX
is a part of the hardware. If we redefine the MCLK[0-4] as "four
clock sources of SAI's clock MUX selecting a clock for bit clock
and frame clock providing" in the binding doc, I feel it'd make
a lot of sense.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2019-04-22  4:25 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-22  3:30 [PATCH] ARM: dts: imx: Add mclk0 clock for SAI S.j. Wang
2019-04-22  3:30 ` S.j. Wang
2019-04-22  4:25 ` Nicolin Chen [this message]
2019-04-22  4:25   ` Nicolin Chen
2019-04-22  4:25   ` Nicolin Chen
  -- strict thread matches above, loose matches on Subject: below --
2019-04-20  9:12 Daniel Baluta
2019-04-20  9:12 ` Daniel Baluta
2019-04-20  9:12 ` Daniel Baluta
2019-04-21  8:20 ` Nicolin Chen
2019-04-21  8:20   ` Nicolin Chen
2019-04-21  8:20   ` Nicolin Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190422042519.GA4304@Asurada \
    --to=nicoleotsuka@gmail.com \
    --cc=aisheng.dong@nxp.com \
    --cc=anson.huang@nxp.com \
    --cc=daniel.baluta@nxp.com \
    --cc=devicetree@vger.kernel.org \
    --cc=festevam@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=peng.fan@nxp.com \
    --cc=robh+dt@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=shengjiu.wang@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.