| Message ID | 1566457123-20791-1-git-send-email-sam.shih@mediatek.com |
|---|---|
| Headers | show |
| Series | Add mt7629 and fix mt7628 pwm | expand |
On Thu, Aug 22, 2019 at 02:58:31PM +0800, Sam Shih wrote: > From: Ryder Lee <ryder.lee@mediatek.com> > > This adds a property "num-pwms" to avoid having an endless > list of compatibles with no differences for the same driver. > > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com> > Signed-off-by: Sam Shih <sam.shih@mediatek.com> Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Thanks Uwe
On Thu, Aug 22, 2019 at 02:58:32PM +0800, Sam Shih wrote: > This patch drop the check for of_device_get_match_data. > Due to the only way call driver probe is compatible match. > The .data pointer which point to the SoC specify data is > directly set by driver, and it should not be NULL in our case. > We can safety remove the check for of_device_get_match_data. > > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com> > Signed-off-by: Sam Shih <sam.shih@mediatek.com> Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Thanks Uwe
Hello Sam, On Thu, Aug 22, 2019 at 02:58:33PM +0800, Sam Shih wrote: > This fix mt7628 pwm during configure from userspace. The SoC > is legacy MIPS and has no complex clock tree. This patch add property > clock-frequency to the SoC specific data and legacy MIPS SoC need to > configure it in DT. This property is use for period calculation. > > We will improve this fix by droping has-clks attribute and using > clock-frequency to do the same thing in a new patch. > > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com> > Signed-off-by: Sam Shih <sam.shih@mediatek.com> I wonder if instead the platform could provide some dummy and optional clocks. You could add a fixed-clock for the clk that is used to determine the clock rate and switch to devm_clk_get_optional for the others. Best regards Uwe
On Thu, Aug 22, 2019 at 02:58:34PM +0800, Sam Shih wrote: > Instead of using fixed size of arrays, allocate the memory for them > based on the information we get from the DT. > > Also remove the check for num_pwms, due to dynamically allocate pwm > should not cause array index out of bound. > > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com> > Signed-off-by: Sam Shih <sam.shih@mediatek.com> Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Thanks Uwe
On Thu, Aug 22, 2019 at 02:58:35PM +0800, Sam Shih wrote: > Use pwm_mediatek as common prefix to match the filename. > No functional change intended. > > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com> > Signed-off-by: Sam Shih <sam.shih@mediatek.com> > --- > Changes since v5: > - Follow reviewers's comments > The license stuff is a separate change this is a nice cleanup, I like it. Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Thanks Uwe
On Thu, Aug 22, 2019 at 02:58:36PM +0800, Sam Shih wrote: > Add SPDX identifiers to pwm-mediatek.c > Update license to GNU General Public License v2.0 > > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com> > Signed-off-by: Sam Shih <sam.shih@mediatek.com> Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Thanks Uwe
On Thu, Aug 22, 2019 at 02:58:39PM +0800, Sam Shih wrote: > From: Ryder Lee <ryder.lee@mediatek.com> > > This adds a property "num-pwms" for PWM controller. > > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com> > Signed-off-by: Sam Shih <sam.shih@mediatek.com> > --- > arch/arm64/boot/dts/mediatek/mt7622.dtsi | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/boot/dts/mediatek/mt7622.dtsi b/arch/arm64/boot/dts/mediatek/mt7622.dtsi > index d1e13d340e26..9a043938881f 100644 > --- a/arch/arm64/boot/dts/mediatek/mt7622.dtsi > +++ b/arch/arm64/boot/dts/mediatek/mt7622.dtsi > @@ -439,6 +439,7 @@ > <&pericfg CLK_PERI_PWM6_PD>; > clock-names = "top", "main", "pwm1", "pwm2", "pwm3", "pwm4", > "pwm5", "pwm6"; > + num-pwms = <6>; > status = "disabled"; > }; FTR: The matching change to the binding is patch 7 in this series and didn't get an Ack from the dt people yet. Best regards Uwe
On Thu, Aug 22, 2019 at 02:58:40PM +0800, Sam Shih wrote: > From: Ryder Lee <ryder.lee@mediatek.com> > > This adds a property "num-pwms" for PWM controller. > > Signed-off-by: Ryder Lee <ryder.lee@mediatek.com> > Signed-off-by: Sam Shih <sam.shih@mediatek.com> > --- > arch/arm/boot/dts/mt7623.dtsi | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/boot/dts/mt7623.dtsi b/arch/arm/boot/dts/mt7623.dtsi > index a79f0b6c3429..208e0d19a575 100644 > --- a/arch/arm/boot/dts/mt7623.dtsi > +++ b/arch/arm/boot/dts/mt7623.dtsi > @@ -452,6 +452,7 @@ > <&pericfg CLK_PERI_PWM5>; > clock-names = "top", "main", "pwm1", "pwm2", > "pwm3", "pwm4", "pwm5"; > + num-pwms = <5>; > status = "disabled"; > }; FTR: The matching change to the binding is patch 7 in this series and didn't get an Ack from the dt people yet. Best regards Uwe
On Thu, Aug 22, 2019 at 02:58:42PM +0800, Sam Shih wrote: > Due to we added clock-frequency property to fix > mt7628 pwm during configure from userspace. > We can alos use this property to determine whether > the complex clock tree exists in the SoC or not. > So we can safety remove has-clock property in the > driver specific data. Some suggestions in short form: s/Due/Since/ s/alos/also/ Also please use more horizontal space, up to 76 chars per line is fine. Other than that I suggest to first address the feedback for the earlier patches as the needed changes there has influence on this patch. Best regards Uwe
Changes since v5: - Follow reviewer's comments: 1. the license stuff is a separate change 2. split fix mt7628 pwm into a single patch 3. to ensure to not use mtk_pwm_clk_name[10] (After dynamic allocate clock array patch, this is no need to check) 4. Use clock-frequency property to replace the use of has_clks Changes since v4: - Follow reviewer's comments (v3: pwm: mediatek: add a property "num-pwms") Move the changes of droping the check for of_device_get_match_data returning non-NULL to next patch - Follow reviewers's comments (v3: pwm: mediatek: allocate the clks array dynamically) 1. use pc->soc->has_clks to check clocks exist or not. 2. Add error message when probe() unable to get clks - Fixes bug when SoC is old mips which has no complex clock tree. if clocks not exist, use the new property from DT to apply period calculation; otherwise, use clk_get_rate to get clock frequency and apply period calculation. Changes since v3: - add a new property "clock-frequency" and fix mt7628 pwm - add mt7629 pwm support Changes since v2: - use num-pwms instead of mediatek,num-pwms. - rename the member from num_pwms to fallback_num_pwms to make it more obvious that it doesn't represent the actually used value. - add a dev_warn and a expressive comment to help other developers to not start adding num_pwms in the compatible_data. Changes since v1: - add some checks for backwards compatibility. Ryder Lee (5): pwm: mediatek: add a property "num-pwms" dt-bindings: pwm: add a property "num-pwms" arm64: dts: mt7622: add a property "num-pwms" for PWM arm: dts: mt7623: add a property "num-pwms" for PWM dt-bindings: pwm: update bindings for MT7629 SoC Sam Shih (8): pwm: mediatek: droping the check for of_device_get_match_data pwm: mediatek: add a property "clock-frequency" pwm: mediatek: allocate the clks array dynamically pwm: mediatek: use pwm_mediatek as common prefix pwm: mediatek: update license and switch to SPDX tag dt-bindings: pwm: update bindings for MT7628 SoC pwm: mediatek: remove a property "has-clock" arm: dts: mediatek: add mt7629 pwm support .../devicetree/bindings/pwm/pwm-mediatek.txt | 12 +- arch/arm/boot/dts/mt7623.dtsi | 1 + arch/arm64/boot/dts/mediatek/mt7622.dtsi | 1 + drivers/pwm/pwm-mediatek.c | 257 ++++++++++-------- arch/arm/boot/dts/mt7629.dtsi | 16 ++++++++++++++++ 5 files changed, 168 insertions(+), 119 deletions(-)