From: Michal Simek <michal.simek@xilinx.com>
To: Sean Anderson <sean.anderson@seco.com>,
<linux-pwm@vger.kernel.org>, <devicetree@vger.kernel.org>
Cc: linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, michal.simek@xilinx.com,
"Alvaro Gamez" <alvaro.gamez@hazent.com>,
"Lee Jones" <lee.jones@linaro.org>,
"Thierry Reding" <thierry.reding@gmail.com>,
"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Subject: Re: [PATCH v2 2/2] pwm: Add support for Xilinx AXI Timer
Date: Wed, 5 May 2021 08:37:59 +0200 [thread overview]
Message-ID: <e3782bc5-bcd9-5eb8-e89b-e4e52ed2e3cb@xilinx.com> (raw)
In-Reply-To: <20210504184925.3399934-2-sean.anderson@seco.com>
On 5/4/21 8:49 PM, Sean Anderson wrote:
> This adds PWM support for Xilinx LogiCORE IP AXI soft timers commonly
> found on Xilinx FPGAs. There is another driver for this device located
> at arch/microblaze/kernel/timer.c, but it is only used for timekeeping.
> This driver was written with reference to Xilinx DS764 for v1.03.a [1].
>
> [1] https://www.xilinx.com/support/documentation/ip_documentation/axi_timer/v1_03_a/axi_timer_ds764.pdf
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
> I tried adding a XILINX_PWM_ prefix to all the defines, but IMO it
> really hurt readability. That prefix almost doubles the size the
> defines, and is particularly excessive in something like
> XILINX_PWM_TCSR_RUN_MASK.
>
> Changes in v2:
> - Don't compile this module by default for arm64
> - Add dependencies on COMMON_CLK and HAS_IOMEM
> - Add comment explaining why we depend on !MICROBLAZE
> - Add comment describing device
> - Rename TCSR_(SET|CLEAR) to TCSR_RUN_(SET|CLEAR)
> - Use NSEC_TO_SEC instead of defining our own
> - Use TCSR_RUN_MASK to check if the PWM is enabled, as suggested by Uwe
> - Cast dividends to u64 to avoid overflow
> - Check for over- and underflow when calculating TLR
> - Set xilinx_pwm_ops.owner
> - Don't set pwmchip.base to -1
> - Check range of xlnx,count-width
> - Ensure the clock is always running when the pwm is registered
> - Remove debugfs file :l
> - Report errors with dev_error_probe
>
> drivers/pwm/Kconfig | 13 ++
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-xilinx.c | 301 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 315 insertions(+)
> create mode 100644 drivers/pwm/pwm-xilinx.c
Without looking below another driver which target the same IP is just
wrong that's why NACK from me.
Thanks,
Michal
WARNING: multiple messages have this Message-ID (diff)
From: Michal Simek <michal.simek@xilinx.com>
To: Sean Anderson <sean.anderson@seco.com>,
<linux-pwm@vger.kernel.org>, <devicetree@vger.kernel.org>
Cc: linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, michal.simek@xilinx.com,
"Alvaro Gamez" <alvaro.gamez@hazent.com>,
"Lee Jones" <lee.jones@linaro.org>,
"Thierry Reding" <thierry.reding@gmail.com>,
"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Subject: Re: [PATCH v2 2/2] pwm: Add support for Xilinx AXI Timer
Date: Wed, 5 May 2021 08:37:59 +0200 [thread overview]
Message-ID: <e3782bc5-bcd9-5eb8-e89b-e4e52ed2e3cb@xilinx.com> (raw)
In-Reply-To: <20210504184925.3399934-2-sean.anderson@seco.com>
On 5/4/21 8:49 PM, Sean Anderson wrote:
> This adds PWM support for Xilinx LogiCORE IP AXI soft timers commonly
> found on Xilinx FPGAs. There is another driver for this device located
> at arch/microblaze/kernel/timer.c, but it is only used for timekeeping.
> This driver was written with reference to Xilinx DS764 for v1.03.a [1].
>
> [1] https://www.xilinx.com/support/documentation/ip_documentation/axi_timer/v1_03_a/axi_timer_ds764.pdf
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
> I tried adding a XILINX_PWM_ prefix to all the defines, but IMO it
> really hurt readability. That prefix almost doubles the size the
> defines, and is particularly excessive in something like
> XILINX_PWM_TCSR_RUN_MASK.
>
> Changes in v2:
> - Don't compile this module by default for arm64
> - Add dependencies on COMMON_CLK and HAS_IOMEM
> - Add comment explaining why we depend on !MICROBLAZE
> - Add comment describing device
> - Rename TCSR_(SET|CLEAR) to TCSR_RUN_(SET|CLEAR)
> - Use NSEC_TO_SEC instead of defining our own
> - Use TCSR_RUN_MASK to check if the PWM is enabled, as suggested by Uwe
> - Cast dividends to u64 to avoid overflow
> - Check for over- and underflow when calculating TLR
> - Set xilinx_pwm_ops.owner
> - Don't set pwmchip.base to -1
> - Check range of xlnx,count-width
> - Ensure the clock is always running when the pwm is registered
> - Remove debugfs file :l
> - Report errors with dev_error_probe
>
> drivers/pwm/Kconfig | 13 ++
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-xilinx.c | 301 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 315 insertions(+)
> create mode 100644 drivers/pwm/pwm-xilinx.c
Without looking below another driver which target the same IP is just
wrong that's why NACK from me.
Thanks,
Michal
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-05-05 6:38 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-04 18:49 [PATCH v2 1/2] dt-bindings: pwm: Add Xilinx AXI Timer Sean Anderson
2021-05-04 18:49 ` Sean Anderson
2021-05-04 18:49 ` [PATCH v2 2/2] pwm: Add support for " Sean Anderson
2021-05-04 18:49 ` Sean Anderson
2021-05-05 6:37 ` Michal Simek [this message]
2021-05-05 6:37 ` Michal Simek
2021-05-06 14:28 ` Sean Anderson
2021-05-06 14:28 ` Sean Anderson
2021-05-06 16:54 ` Michal Simek
2021-05-06 16:54 ` Michal Simek
2021-05-06 22:36 ` Sean Anderson
2021-05-06 22:36 ` Sean Anderson
2021-05-10 10:20 ` Michal Simek
2021-05-10 10:20 ` Michal Simek
2021-05-11 19:16 ` Sean Anderson
2021-05-11 19:16 ` Sean Anderson
2021-05-05 6:46 ` [PATCH v2 1/2] dt-bindings: pwm: Add " Michal Simek
2021-05-05 6:46 ` Michal Simek
2021-05-06 14:24 ` Sean Anderson
2021-05-06 14:24 ` Sean Anderson
2021-05-06 21:05 ` Rob Herring
2021-05-06 21:05 ` Rob Herring
2021-05-06 21:10 ` Sean Anderson
2021-05-06 21:10 ` Sean Anderson
2021-05-07 6:35 ` Michal Simek
2021-05-07 6:35 ` Michal Simek
2021-05-07 14:24 ` Sean Anderson
2021-05-07 14:24 ` Sean Anderson
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=e3782bc5-bcd9-5eb8-e89b-e4e52ed2e3cb@xilinx.com \
--to=michal.simek@xilinx.com \
--cc=alvaro.gamez@hazent.com \
--cc=devicetree@vger.kernel.org \
--cc=lee.jones@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pwm@vger.kernel.org \
--cc=sean.anderson@seco.com \
--cc=thierry.reding@gmail.com \
--cc=u.kleine-koenig@pengutronix.de \
/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.