All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tony Lindgren <tony@atomide.com>
To: Peter Ujfalusi <peter.ujfalusi@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>,
	jarkko.nikula@bitmer.com, t-kristo@ti.com,
	linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v2 0/3] ARM: OMAP3: Fix McBSP2/3 hwmod setup for sidetone
Date: Thu, 14 Apr 2016 13:34:58 -0700	[thread overview]
Message-ID: <20160414203457.GM5995@atomide.com> (raw)
In-Reply-To: <570FF163.1050603@ti.com>

* Peter Ujfalusi <peter.ujfalusi@ti.com> [160414 12:38]:
> 
> Yes it has registers, but it has no prcm level existence, it is part of McBSP
> module. I guess when the OMAP3 was designed the HW people did not wanted to
> create new version of the McBSP core for McBSP2/3 so they attached a new core
> to the McBSP cores with different targets, etc, but w/o external dependency.

Yeah well we do have a bunch of modules that don't need any separate
functional clock and are clocked only by the interface clock. So in this
case McBSP and sidetone are both consumers for the clock we just happen
to call McBSP interface clock. They should be able to share that no
problem.

> >> OK. I will go with the assumption that the sidetone hwmod can be removed (as
> >> it is not correct) and rework my current series to use pdata callback for the
> >> iclk autogate allow/deny. With this set the ST will be operational in legacy
> >> and DT boot.
> > 
> > Sorry, no I did not want to drop the sidetone hwmod, I was just trying to
> > come up with ideas on how to make the driver changes easier. It sounds like
> > you already figured out the driver changes part though with two drivers.
> 
> If I need to keep the sidetone hwmod around I don't see how it can be done in
> a safe and clean way. It is part of McBSP module, it is accessible only if the
> McBSP module is enabled, you can not enable the Sidetone alone you need to go
> and enable the McBSP module. I don't think it is a good idea to let two
> separate hwmods to poke around the same PRCM bits. Have not checked, but I
> don't think we have refcounting for the PRCM register bits.

Yeah there's no refcounting on the PRCM, but the clock framework has it
for the share McBSP interface clock. Then there are two separate sets of
sysconfig registers that PM runtime should manage. And then there's the
clock autogating issue.

Note that we do have an issue with the omap4 and later clkctrl registers
that don't have refcounting. Tero's clkctrl work will sort out that
issue. I don't think we have a similar issue with omap3.

So from that point of view the two separate hwmod modules should work
just fine sharing the clock.

> I have things working w/o the two drivers with pdata callback both in legacy
> and DT case and it is pretty neat looking, thanks for the suggestion! I'm
> still figuring out the needed amount of callbacks from McBSP to ST and from ST
> to McBSP. We for sure going to need enable_stage1,2 probably three as well.
> But this crossing driver boundaries needs a bit more time to figure out.

Yeah I still think we need to struct device instances, one to manage
McBSP and the other to manage sidetone :)

Regards,

Tony

WARNING: multiple messages have this Message-ID (diff)
From: tony@atomide.com (Tony Lindgren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 0/3] ARM: OMAP3: Fix McBSP2/3 hwmod setup for sidetone
Date: Thu, 14 Apr 2016 13:34:58 -0700	[thread overview]
Message-ID: <20160414203457.GM5995@atomide.com> (raw)
In-Reply-To: <570FF163.1050603@ti.com>

* Peter Ujfalusi <peter.ujfalusi@ti.com> [160414 12:38]:
> 
> Yes it has registers, but it has no prcm level existence, it is part of McBSP
> module. I guess when the OMAP3 was designed the HW people did not wanted to
> create new version of the McBSP core for McBSP2/3 so they attached a new core
> to the McBSP cores with different targets, etc, but w/o external dependency.

Yeah well we do have a bunch of modules that don't need any separate
functional clock and are clocked only by the interface clock. So in this
case McBSP and sidetone are both consumers for the clock we just happen
to call McBSP interface clock. They should be able to share that no
problem.

> >> OK. I will go with the assumption that the sidetone hwmod can be removed (as
> >> it is not correct) and rework my current series to use pdata callback for the
> >> iclk autogate allow/deny. With this set the ST will be operational in legacy
> >> and DT boot.
> > 
> > Sorry, no I did not want to drop the sidetone hwmod, I was just trying to
> > come up with ideas on how to make the driver changes easier. It sounds like
> > you already figured out the driver changes part though with two drivers.
> 
> If I need to keep the sidetone hwmod around I don't see how it can be done in
> a safe and clean way. It is part of McBSP module, it is accessible only if the
> McBSP module is enabled, you can not enable the Sidetone alone you need to go
> and enable the McBSP module. I don't think it is a good idea to let two
> separate hwmods to poke around the same PRCM bits. Have not checked, but I
> don't think we have refcounting for the PRCM register bits.

Yeah there's no refcounting on the PRCM, but the clock framework has it
for the share McBSP interface clock. Then there are two separate sets of
sysconfig registers that PM runtime should manage. And then there's the
clock autogating issue.

Note that we do have an issue with the omap4 and later clkctrl registers
that don't have refcounting. Tero's clkctrl work will sort out that
issue. I don't think we have a similar issue with omap3.

So from that point of view the two separate hwmod modules should work
just fine sharing the clock.

> I have things working w/o the two drivers with pdata callback both in legacy
> and DT case and it is pretty neat looking, thanks for the suggestion! I'm
> still figuring out the needed amount of callbacks from McBSP to ST and from ST
> to McBSP. We for sure going to need enable_stage1,2 probably three as well.
> But this crossing driver boundaries needs a bit more time to figure out.

Yeah I still think we need to struct device instances, one to manage
McBSP and the other to manage sidetone :)

Regards,

Tony

  reply	other threads:[~2016-04-14 20:35 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-18 14:23 [PATCH v2 0/3] ARM: OMAP3: Fix McBSP2/3 hwmod setup for sidetone Peter Ujfalusi
2016-03-18 14:23 ` Peter Ujfalusi
2016-03-18 14:23 ` Peter Ujfalusi
2016-03-18 14:23 ` [PATCH v2 1/3] ARM: DTS: omap3: Remove mcbsp2/3_sidetone hwmod reference for McBSP2/3 Peter Ujfalusi
2016-03-18 14:23   ` Peter Ujfalusi
2016-03-18 14:23   ` Peter Ujfalusi
2016-03-18 14:23 ` [PATCH v2 2/3] ARM: OMAP2+: mcbsp: Prepare the device build code for sidetone hwmod removal Peter Ujfalusi
2016-03-18 14:23   ` Peter Ujfalusi
2016-03-18 14:23   ` Peter Ujfalusi
2016-03-18 14:23 ` [PATCH v2 3/3] ARM: OMAP3: hwmod data: Merge and remove the McBSP sidetone related data Peter Ujfalusi
2016-03-18 14:23   ` Peter Ujfalusi
2016-03-18 14:23   ` Peter Ujfalusi
2016-03-19 19:38 ` [PATCH v2 0/3] ARM: OMAP3: Fix McBSP2/3 hwmod setup for sidetone Paul Walmsley
2016-03-19 19:38   ` Paul Walmsley
2016-03-19 19:38   ` Paul Walmsley
2016-03-21  8:57   ` Peter Ujfalusi
2016-03-21  8:57     ` Peter Ujfalusi
2016-03-21  8:57     ` Peter Ujfalusi
2016-03-21 17:44     ` Paul Walmsley
2016-03-21 17:44       ` Paul Walmsley
2016-04-01  9:33       ` Peter Ujfalusi
2016-04-01  9:33         ` Peter Ujfalusi
2016-04-01  9:33         ` Peter Ujfalusi
2016-04-02  0:17         ` Tony Lindgren
2016-04-02  0:17           ` Tony Lindgren
2016-04-02  0:17           ` Tony Lindgren
2016-04-04 12:45           ` Peter Ujfalusi
2016-04-04 12:45             ` Peter Ujfalusi
2016-04-04 12:45             ` Peter Ujfalusi
2016-04-04 15:12             ` Tony Lindgren
2016-04-04 15:12               ` Tony Lindgren
2016-04-05 13:15               ` Peter Ujfalusi
2016-04-05 13:15                 ` Peter Ujfalusi
2016-04-05 13:15                 ` Peter Ujfalusi
2016-04-11 21:28                 ` Tony Lindgren
2016-04-11 21:28                   ` Tony Lindgren
2016-04-11 21:28                   ` Tony Lindgren
2016-04-12  9:52                   ` Peter Ujfalusi
2016-04-12  9:52                     ` Peter Ujfalusi
2016-04-12  9:52                     ` Peter Ujfalusi
2016-04-12 16:37                     ` Tony Lindgren
2016-04-12 16:37                       ` Tony Lindgren
2016-04-13 11:57                       ` Peter Ujfalusi
2016-04-13 11:57                         ` Peter Ujfalusi
2016-04-13 11:57                         ` Peter Ujfalusi
2016-04-13 15:28                         ` Tony Lindgren
2016-04-13 15:28                           ` Tony Lindgren
2016-04-14  7:34                           ` Peter Ujfalusi
2016-04-14  7:34                             ` Peter Ujfalusi
2016-04-14  7:34                             ` Peter Ujfalusi
2016-04-14 16:55                             ` Tony Lindgren
2016-04-14 16:55                               ` Tony Lindgren
2016-04-14 19:37                               ` Peter Ujfalusi
2016-04-14 19:37                                 ` Peter Ujfalusi
2016-04-14 19:37                                 ` Peter Ujfalusi
2016-04-14 20:34                                 ` Tony Lindgren [this message]
2016-04-14 20:34                                   ` Tony Lindgren
2016-04-15 10:23                                   ` Peter Ujfalusi
2016-04-15 10:23                                     ` Peter Ujfalusi
2016-04-15 10:23                                     ` Peter Ujfalusi
2016-04-15 15:16                                     ` Tony Lindgren
2016-04-15 15:16                                       ` Tony Lindgren
2016-04-15 15:16                                       ` Tony Lindgren
2016-04-15 19:50                                       ` Peter Ujfalusi
2016-04-15 19:50                                         ` Peter Ujfalusi
2016-04-15 19:50                                         ` Peter Ujfalusi
2016-04-18 23:51                                         ` Tony Lindgren
2016-04-18 23:51                                           ` Tony Lindgren
2016-04-22 13:12                                           ` Peter Ujfalusi
2016-04-22 13:12                                             ` Peter Ujfalusi
2016-04-22 13:12                                             ` Peter Ujfalusi
2016-04-22 22:24                                             ` Tony Lindgren
2016-04-22 22:24                                               ` Tony Lindgren
2016-04-22 22:24                                               ` Tony Lindgren

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=20160414203457.GM5995@atomide.com \
    --to=tony@atomide.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jarkko.nikula@bitmer.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=paul@pwsan.com \
    --cc=peter.ujfalusi@ti.com \
    --cc=t-kristo@ti.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.