linux-kernel - Re: [PATCH 2/2] pwm: Add support for Xilinx AXI Timer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <71694d6a-21d8-2b31-0e66-2dfea52a6390@seco.com>
Date:   Tue, 4 May 2021 11:57:20 -0400
From:   Sean Anderson <sean.anderson@...o.com>
To:     Michal Simek <michal.simek@...inx.com>,
        Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
Cc:     linux-pwm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        Lee Jones <lee.jones@...aro.org>,
        Thierry Reding <thierry.reding@...il.com>
Subject: Re: [PATCH 2/2] pwm: Add support for Xilinx AXI Timer



On 5/4/21 8:32 AM, Michal Simek wrote:
 >
 >
 > On 5/4/21 10:51 AM, Uwe Kleine-König wrote:
 >> Hello,
 >>
 >> On Mon, May 03, 2021 at 05:44:13PM -0400, 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@...o.com>
 >>> ---
 >>>
 >>>   arch/arm64/configs/defconfig |   1 +
 >>>   drivers/pwm/Kconfig          |  11 ++
 >>>   drivers/pwm/Makefile         |   1 +
 >>>   drivers/pwm/pwm-xilinx.c     | 322 +++++++++++++++++++++++++++++++++++
 >>>   4 files changed, 335 insertions(+)
 >>>   create mode 100644 drivers/pwm/pwm-xilinx.c
 >>>
 >>> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
 >>> index 08c6f769df9a..81794209f287 100644
 >>> --- a/arch/arm64/configs/defconfig
 >>> +++ b/arch/arm64/configs/defconfig
 >>> @@ -1083,6 +1083,7 @@ CONFIG_PWM_SAMSUNG=y
 >>>   CONFIG_PWM_SL28CPLD=m
 >>>   CONFIG_PWM_SUN4I=m
 >>>   CONFIG_PWM_TEGRA=m
 >>> +CONFIG_PWM_XILINX=m
 >>>   CONFIG_SL28CPLD_INTC=y
 >>>   CONFIG_QCOM_PDC=y
 >>>   CONFIG_RESET_IMX7=y
 >>
 >> I think this should go into a separate patch once this driver is
 >> accepted. This can then go via the ARM people.
 >>
 >>> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
 >>> index d3371ac7b871..01e62928f4bf 100644
 >>> --- a/drivers/pwm/Kconfig
 >>> +++ b/drivers/pwm/Kconfig
 >>> @@ -628,4 +628,15 @@ config PWM_VT8500
 >>>   	  To compile this driver as a module, choose M here: the module
 >>>   	  will be called pwm-vt8500.
 >>>
 >>> +config PWM_XILINX
 >>> +	tristate "Xilinx AXI Timer PWM support"
 >>> +	depends on !MICROBLAZE
 >>
 >> I don't understand this dependency.
 >
 > The dependency is clear here because microblaze has already driver for
 > this timer here arch/microblaze/kernel/timer.c.
 >
 > And that's exactly pointing to the way how this should be done.
 > IP itself is single or dual timer and in case of dual timer you can
 > select if there is pwm output and use it for PWM generation.
 >
 > It means it is timer with PMW together.
 > I didn't have a time but Uwe likely knows this better how to design it.
 >
 > I see that gpio-mvebu driver instantiate pwm driver. Maybe that's the
 > way to go.

I think drivers/clocksource/samsung_pwm_timer.c and
drivers/pwm/pwm-samsung.c provide another example for how to go about
this.

 > Step first is move axi timer driver from microblaze to generic location.

Yes. However, I don't have a microblaze setup, so I have just added the
PWM driver.

 > Figured it out how to add PWM (with DT flag) and then write support for it.

I would really like to see a standard way of doing this. Many timers
also have PWM support (e.g. samsung, and also DW). I think it's unusual
that this problem has not already been addressed.

--Sean

 >
 > Thanks,
 > Michal
 >