On Thu, Apr 13, 2017 at 07:40:28PM +0530, Laxman Dewangan wrote:
> The PWM hardware IP is taped-out with different maximum frequency
> on different SoCs.
>
> From HW team:
> For Tegra210, it is 38.4MHz.
> For Tegra186, it is 102MHz.
>
> Add support to limit the clock source frequency to the maximum IP
> supported frequency. Provide these values via SoC chipdata.
>
> Signed-off-by: Laxman Dewangan <ldewangan@xxxxxxxxxx>
> ---
> drivers/pwm/pwm-tegra.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> index 8c6ed55..7016c08 100644
> --- a/drivers/pwm/pwm-tegra.c
> +++ b/drivers/pwm/pwm-tegra.c
> @@ -41,6 +41,9 @@
>
> struct tegra_pwm_soc {
> unsigned int num_channels;
> +
> + /* Maximum IP frequency for given SoCs */
> + unsigned long max_frequency;
> };
>
> struct tegra_pwm_chip {
> @@ -204,6 +207,24 @@ static int tegra_pwm_probe(struct platform_device *pdev)
> /* Read PWM clock rate from source */
> pwm->clk_rate = clk_get_rate(pwm->clk);
>
> + /* Make sure clock source freqeuncy must less than IP supported */
> + if (pwm->soc->max_frequency &&
> + (pwm->soc->max_frequency < pwm->clk_rate)) {
> + ret = clk_set_rate(pwm->clk, pwm->soc->max_frequency);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Failed to set max frequency: %d\n",
> + ret);
> + return ret;
> + }
> +
> + /*
> + * The requested and configured frequency may differ due to
> + * clock register resolutions. Get the configured frequency
> + * so that PWM period can be calculated more accurately.
> + */
> + pwm->clk_rate = clk_get_rate(pwm->clk);
> + }
Is there a reason to conditionalize this? Couldn't we simply set the
clock to the maximum frequency in all cases? Higher frequency means
higher precision, right? So just something like this perhaps:
ret = clk_set_rate(pwm->clk, pwm->soc->max_frequency);
if (ret < 0) {
...
}
pwm->clk_rate = clk_get_rate(pwm->clk);
That of course means that we'd need to define a maximum frequency for
SoCs prior to Tegra210. Any chance we can get at them?
> +
> pwm->rst = devm_reset_control_get(&pdev->dev, "pwm");
> if (IS_ERR(pwm->rst)) {
> ret = PTR_ERR(pwm->rst);
> @@ -275,12 +296,19 @@ static const struct tegra_pwm_soc tegra20_pwm_soc = {
> .num_channels = 4,
> };
>
> +static const struct tegra_pwm_soc tegra210_pwm_soc = {
> + .num_channels = 4,
> + .max_frequency = 38400000UL, /* 38.4MHz */
> +};
> +
> static const struct tegra_pwm_soc tegra186_pwm_soc = {
> .num_channels = 1,
> + .max_frequency = 102000000UL, /* 102MHz */
I don't think we need these comments, it's fairly obvious what
frequencies you're specifying there. =)
Thierry
Attachment:
signature.asc
Description: PGP signature
