All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Franklin S Cooper Jr." <fcooper@ti.com>
To: Tony Lindgren <tony@atomide.com>
Cc: <paul@pwsan.com>, <t-kristo@ti.com>, <vigneshr@ti.com>,
	<linux-pwm@vger.kernel.org>, <devicetree@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-omap@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-clk@vger.kernel.org>
Subject: Re: [PATCH v3 1/5] pwms: pwm-ti*: Remove support for local clock gating
Date: Mon, 29 Feb 2016 16:30:53 -0600	[thread overview]
Message-ID: <56D4C69D.1090100@ti.com> (raw)
In-Reply-To: <20160229220444.GQ13417@atomide.com>

Hi Tony,

On 02/29/2016 04:04 PM, Tony Lindgren wrote:
> * Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]:
>> The PWMSS local clock gating registers have no real purpose on OMAP ARM
>> devices. These registers were left over registers from DSP IP where the
>> PRCM doesn't exist. There is a silicon bug where gating and ungating clocks
>> don't function properly. TRMs will be update to indicate that these
>> registers shouldn't be touched.
>>
>> Therefore, all code that accesses the PWMSS_CLKCONFIG or PWMSS_CLKSTATUS
>> will be removed by this patch with zero loss of functionality by the ECAP
>> and EPWM drivers.
>> @@ -243,40 +240,15 @@ static int ecap_pwm_probe(struct platform_device *pdev)
>>  	}
>>  
>>  	pm_runtime_enable(&pdev->dev);
>> -	pm_runtime_get_sync(&pdev->dev);
>> -
>> -	status = pwmss_submodule_state_change(pdev->dev.parent,
>> -			PWMSS_ECAPCLK_EN);
>> -	if (!(status & PWMSS_ECAPCLK_EN_ACK)) {
>> -		dev_err(&pdev->dev, "PWMSS config space clock enable failed\n");
>> -		ret = -EINVAL;
>> -		goto pwmss_clk_failure;
>> -	}
>> -
>> -	pm_runtime_put_sync(&pdev->dev);
>>  
>>  	platform_set_drvdata(pdev, pc);
>>  	return 0;
>> -
>> -pwmss_clk_failure:
>> -	pm_runtime_put_sync(&pdev->dev);
>> -	pm_runtime_disable(&pdev->dev);
>> -	pwmchip_remove(&pc->chip);
>> -	return ret;
>>  }
> Hmm but why are you also removing the pm_runtime calls? Those
> actually do take care of gating the clocks via the interconnect
> level code that is hwmod in this case.
I removed all PM runtime calls that revolved around
pwmss_submodule_state_change. Originally the driver would do
a pm_runtime_get_sync then call pwmss_submodule_state_change
and then immediately call pm_runtime_put_sync. Without
pwmss_submodule_state_change those calls would be
meaningless.  I also removed pm_runtime calls in error paths
that no longer existed.

Within ecap and epwm driver pm_runtime_get is done when
pwm_enable is called  and pm_runtime_sync calls are done
when pwm_disable is called. Similar pm_runtime_get and
pm_runtime_sync calls are done in functions that ended up
touching register's within the IP.
>
> Regards,
>
> Tony


WARNING: multiple messages have this Message-ID (diff)
From: "Franklin S Cooper Jr." <fcooper@ti.com>
To: Tony Lindgren <tony@atomide.com>
Cc: paul@pwsan.com, t-kristo@ti.com, vigneshr@ti.com,
	linux-pwm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org
Subject: Re: [PATCH v3 1/5] pwms: pwm-ti*: Remove support for local clock gating
Date: Mon, 29 Feb 2016 16:30:53 -0600	[thread overview]
Message-ID: <56D4C69D.1090100@ti.com> (raw)
In-Reply-To: <20160229220444.GQ13417@atomide.com>

Hi Tony,

On 02/29/2016 04:04 PM, Tony Lindgren wrote:
> * Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]:
>> The PWMSS local clock gating registers have no real purpose on OMAP ARM
>> devices. These registers were left over registers from DSP IP where the
>> PRCM doesn't exist. There is a silicon bug where gating and ungating clocks
>> don't function properly. TRMs will be update to indicate that these
>> registers shouldn't be touched.
>>
>> Therefore, all code that accesses the PWMSS_CLKCONFIG or PWMSS_CLKSTATUS
>> will be removed by this patch with zero loss of functionality by the ECAP
>> and EPWM drivers.
>> @@ -243,40 +240,15 @@ static int ecap_pwm_probe(struct platform_device *pdev)
>>  	}
>>  
>>  	pm_runtime_enable(&pdev->dev);
>> -	pm_runtime_get_sync(&pdev->dev);
>> -
>> -	status = pwmss_submodule_state_change(pdev->dev.parent,
>> -			PWMSS_ECAPCLK_EN);
>> -	if (!(status & PWMSS_ECAPCLK_EN_ACK)) {
>> -		dev_err(&pdev->dev, "PWMSS config space clock enable failed\n");
>> -		ret = -EINVAL;
>> -		goto pwmss_clk_failure;
>> -	}
>> -
>> -	pm_runtime_put_sync(&pdev->dev);
>>  
>>  	platform_set_drvdata(pdev, pc);
>>  	return 0;
>> -
>> -pwmss_clk_failure:
>> -	pm_runtime_put_sync(&pdev->dev);
>> -	pm_runtime_disable(&pdev->dev);
>> -	pwmchip_remove(&pc->chip);
>> -	return ret;
>>  }
> Hmm but why are you also removing the pm_runtime calls? Those
> actually do take care of gating the clocks via the interconnect
> level code that is hwmod in this case.
I removed all PM runtime calls that revolved around
pwmss_submodule_state_change. Originally the driver would do
a pm_runtime_get_sync then call pwmss_submodule_state_change
and then immediately call pm_runtime_put_sync. Without
pwmss_submodule_state_change those calls would be
meaningless.  I also removed pm_runtime calls in error paths
that no longer existed.

Within ecap and epwm driver pm_runtime_get is done when
pwm_enable is called  and pm_runtime_sync calls are done
when pwm_disable is called. Similar pm_runtime_get and
pm_runtime_sync calls are done in functions that ended up
touching register's within the IP.
>
> Regards,
>
> Tony

WARNING: multiple messages have this Message-ID (diff)
From: fcooper@ti.com (Franklin S Cooper Jr.)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 1/5] pwms: pwm-ti*: Remove support for local clock gating
Date: Mon, 29 Feb 2016 16:30:53 -0600	[thread overview]
Message-ID: <56D4C69D.1090100@ti.com> (raw)
In-Reply-To: <20160229220444.GQ13417@atomide.com>

Hi Tony,

On 02/29/2016 04:04 PM, Tony Lindgren wrote:
> * Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]:
>> The PWMSS local clock gating registers have no real purpose on OMAP ARM
>> devices. These registers were left over registers from DSP IP where the
>> PRCM doesn't exist. There is a silicon bug where gating and ungating clocks
>> don't function properly. TRMs will be update to indicate that these
>> registers shouldn't be touched.
>>
>> Therefore, all code that accesses the PWMSS_CLKCONFIG or PWMSS_CLKSTATUS
>> will be removed by this patch with zero loss of functionality by the ECAP
>> and EPWM drivers.
>> @@ -243,40 +240,15 @@ static int ecap_pwm_probe(struct platform_device *pdev)
>>  	}
>>  
>>  	pm_runtime_enable(&pdev->dev);
>> -	pm_runtime_get_sync(&pdev->dev);
>> -
>> -	status = pwmss_submodule_state_change(pdev->dev.parent,
>> -			PWMSS_ECAPCLK_EN);
>> -	if (!(status & PWMSS_ECAPCLK_EN_ACK)) {
>> -		dev_err(&pdev->dev, "PWMSS config space clock enable failed\n");
>> -		ret = -EINVAL;
>> -		goto pwmss_clk_failure;
>> -	}
>> -
>> -	pm_runtime_put_sync(&pdev->dev);
>>  
>>  	platform_set_drvdata(pdev, pc);
>>  	return 0;
>> -
>> -pwmss_clk_failure:
>> -	pm_runtime_put_sync(&pdev->dev);
>> -	pm_runtime_disable(&pdev->dev);
>> -	pwmchip_remove(&pc->chip);
>> -	return ret;
>>  }
> Hmm but why are you also removing the pm_runtime calls? Those
> actually do take care of gating the clocks via the interconnect
> level code that is hwmod in this case.
I removed all PM runtime calls that revolved around
pwmss_submodule_state_change. Originally the driver would do
a pm_runtime_get_sync then call pwmss_submodule_state_change
and then immediately call pm_runtime_put_sync. Without
pwmss_submodule_state_change those calls would be
meaningless.  I also removed pm_runtime calls in error paths
that no longer existed.

Within ecap and epwm driver pm_runtime_get is done when
pwm_enable is called  and pm_runtime_sync calls are done
when pwm_disable is called. Similar pm_runtime_get and
pm_runtime_sync calls are done in functions that ended up
touching register's within the IP.
>
> Regards,
>
> Tony

  reply	other threads:[~2016-02-29 22:31 UTC|newest]

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-25 22:36 [PATCH v3 0/5] Add support for PWMSS on DRA7 Franklin S Cooper Jr
2016-02-25 22:36 ` Franklin S Cooper Jr
2016-02-25 22:36 ` Franklin S Cooper Jr
2016-02-25 22:36 ` [PATCH v3 1/5] pwms: pwm-ti*: Remove support for local clock gating Franklin S Cooper Jr
2016-02-25 22:36   ` Franklin S Cooper Jr
2016-02-26 10:27   ` Sekhar Nori
2016-02-26 10:27     ` Sekhar Nori
2016-02-26 10:27     ` Sekhar Nori
2016-02-26 19:14     ` Tony Lindgren
2016-02-26 19:14       ` Tony Lindgren
2016-02-29 22:04   ` Tony Lindgren
2016-02-29 22:04     ` Tony Lindgren
2016-02-29 22:04     ` Tony Lindgren
2016-02-29 22:30     ` Franklin S Cooper Jr. [this message]
2016-02-29 22:30       ` Franklin S Cooper Jr.
2016-02-29 22:30       ` Franklin S Cooper Jr.
2016-02-29 22:55       ` Tony Lindgren
2016-02-29 22:55         ` Tony Lindgren
2016-02-29 23:11         ` Franklin S Cooper Jr.
2016-02-29 23:11           ` Franklin S Cooper Jr.
2016-02-29 23:11           ` Franklin S Cooper Jr.
2016-02-29 23:20           ` Tony Lindgren
2016-02-29 23:20             ` Tony Lindgren
2016-03-02 19:41             ` Franklin S Cooper Jr.
2016-03-02 19:41               ` Franklin S Cooper Jr.
2016-03-02 19:41               ` Franklin S Cooper Jr.
2016-03-02 22:54               ` Tony Lindgren
2016-03-02 22:54                 ` Tony Lindgren
2016-02-25 22:36 ` [PATCH v3 2/5] ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS Franklin S Cooper Jr
2016-02-25 22:36   ` Franklin S Cooper Jr
2016-03-01 18:07   ` Paul Walmsley
2016-03-01 18:07     ` Paul Walmsley
2016-03-01 18:07     ` Paul Walmsley
2016-03-01 18:11   ` Tony Lindgren
2016-03-01 18:11     ` Tony Lindgren
2016-03-01 18:59     ` Paul Walmsley
2016-03-01 18:59       ` Paul Walmsley
2016-03-01 18:59       ` Paul Walmsley
2016-03-01 20:50       ` Tony Lindgren
2016-03-01 20:50         ` Tony Lindgren
2016-03-02 16:22         ` Franklin S Cooper Jr.
2016-03-02 16:22           ` Franklin S Cooper Jr.
2016-03-02 16:22           ` Franklin S Cooper Jr.
2016-03-04  2:07           ` Franklin S Cooper Jr.
2016-03-04  2:07             ` Franklin S Cooper Jr.
2016-03-04  2:07             ` Franklin S Cooper Jr.
2016-03-04  6:25             ` Paul Walmsley
2016-03-04  6:25               ` Paul Walmsley
2016-03-04 12:23               ` Franklin S Cooper Jr.
2016-03-04 12:23                 ` Franklin S Cooper Jr.
2016-03-04 12:23                 ` Franklin S Cooper Jr.
2016-03-04 16:32                 ` Paul Walmsley
2016-03-04 16:32                   ` Paul Walmsley
2016-02-25 22:36 ` [PATCH v3 3/5] ARM: dts: DRA7: Add TBCLK " Franklin S Cooper Jr
2016-02-25 22:36   ` Franklin S Cooper Jr
2016-02-29 23:23   ` Tony Lindgren
2016-02-29 23:23     ` Tony Lindgren
2016-03-01 12:54     ` Tero Kristo
2016-03-01 12:54       ` Tero Kristo
2016-03-01 12:54       ` Tero Kristo
2016-03-01 18:05       ` Tony Lindgren
2016-03-01 18:05         ` Tony Lindgren
2016-02-25 22:36 ` [PATCH v3 4/5] clk: ti: DRA7: Add tbclk data for ehrpwm Franklin S Cooper Jr
2016-02-25 22:36   ` Franklin S Cooper Jr
2016-02-26 19:16   ` Tony Lindgren
2016-02-26 19:16     ` Tony Lindgren
2016-02-26 19:17     ` Tony Lindgren
2016-02-26 19:17       ` Tony Lindgren
2016-02-25 22:36 ` [PATCH v3 5/5] ARM: dts: DRA7: Add dt nodes for PWMSS Franklin S Cooper Jr
2016-02-25 22:36   ` Franklin S Cooper Jr
2016-02-26 19:18   ` Tony Lindgren
2016-02-26 19:18     ` Tony Lindgren
2016-02-26 19:43     ` Franklin S Cooper Jr
2016-02-26 19:43       ` Franklin S Cooper Jr
2016-02-26 19:43       ` Franklin S Cooper Jr
2016-02-29 23:24   ` Tony Lindgren
2016-02-29 23:24     ` Tony Lindgren
2016-03-01 21:00     ` Tony Lindgren
2016-03-01 21:00       ` Tony Lindgren
2016-03-02 18:26   ` Rob Herring
2016-03-02 18:26     ` Rob Herring
2016-03-04  1:39     ` Franklin S Cooper Jr.
2016-03-04  1:39       ` Franklin S Cooper Jr.
2016-03-04  1:39       ` Franklin S Cooper Jr.
2016-03-04 14:52       ` Rob Herring
2016-03-04 14:52         ` Rob Herring

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=56D4C69D.1090100@ti.com \
    --to=fcooper@ti.com \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=paul@pwsan.com \
    --cc=t-kristo@ti.com \
    --cc=tony@atomide.com \
    --cc=vigneshr@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.