| Message ID | cover.1603350036.git.vijayakannan.ayyathurai@intel.com |
|---|---|
| Headers | show |
| Series | Add PWM support for Intel Keem Bay SoC | expand |
On Thu, Oct 22, 2020 at 03:14:45PM +0800, vijayakannan.ayyathurai@intel.com wrote: > From: Vijayakannan Ayyathurai <vijayakannan.ayyathurai@intel.com> > > Hi, > > This patch set enables the support for PWM in the Intel Keem Bay SoC. > Keem Bay is an ARM based SoC, and the GPIO module allows > configuration of 6 PWM outputs. > > Patch 1 adds the PWM driver and Patch 2 is for the required > Device Tree bindings documentation. > > This driver was tested on the Keem Bay evaluation module board. > > Thank you. > > Regards, > Vijay > > Changes since v13: > - Fix indentation error in dt-binding. > - Add maintainer name in dt-binding. > > Changes since v12: > - Use devm_add_action_or_reset() as per Andy suggestion. > - Do the clk_prepare_enable() after all devm_ stuff as per Uwe suggestion. > - Optimize keembay_pwm_remove function. > - Simplify the error handling for pwmchip_add. > - In Kconfig, Use depends on ARCH_KEEMBAY || (ARM64 && COMPILE_TEST) > - Fix indentation error in dt-binding. > - Add Uwe's Reviewed-by tag and Vijay's Co-developed-by tag. > > Changes since v11: > - Minor variable name change to match the api needs. > - Setup polarity as PWM_POLARITY_NORMAL. > - Add comment for LEADIN register usage. > > Changes since v10: > - Update low time calculation formula as per Uwe. > - During distruct remove pwmchip first then disable the clock. > > Changes since v9: > - Remove Reported-by tag from the commit log. > > Changes since v8: > - Fix the compilation error reported by kernel test robot. > - Add the tag Reported-by: kernel test robot <lkp@intel.com> > - Minor correction in the pwm low time calculation formula. > - Rebase with 5.9-rc7 > > Changes since v7: > - Change the dependency as ARCH_KEEMBAY instead of ARM64 in Kconfig. > - Use DIV_ROUND_DOWN_ULL instead of DIV_ROUND_CLOSEST_ULL. > - Update the right formula as per Uwe. > - List the tags in chronological order. > - Add clk_disable_unprepare in the error paths. > > Changes since v6: > - Add reviewed-by tag > > Changes since v5: > - Reorder symbols/Kconfig in drivers/pwm/Kconfig and drivers/pwm/Makefile > - Use "Limitations" for consistency > - Add clk_prepare_enable() > - Reorder keembay_pwm_get_state() function call > - Rework if conditional for channel disablement in .apply() > - Remove channel disabling from .probe(), and clear LEADIN register bits > in .apply instead > - Update commit message for Patch 1 > > Changes since v4: > - Add co-developed-by tag > - Include mod_devicetable.h and remove of.h > - Update comment with correct calulation for high/low time > - Fix missing return from dev_err_probe > > Changes since v3: > - Removed variable for address and calculate in place instead > - Utilized u32_replace_bits() when updating KMB_PWM_LEADIN_OFFSET > - Utilized dev_err_probe() for error reporting > - Updated comments to use physical units > - Updated error check for pwmchip_add() > > Changes since v2: > - Include documentation about HW limitation/behaviour > - Use hex values for KMB_PWM_COUNT_MAX > - Redefine register macros > - Utilize FIELD_GET/FIELD_PREP for calculating pwm_l/h_count and > pwm_count > - Round up duty cycle/period values > - Get current hardware state in .apply instead of cached values > - Do a polarity check before .enabled > - Round high time/low time to closest value > - Set enable bit in KMB_PWM_LEADIN_OFFSET to 0 in probe > - Correct the naming for MODULE_ALIAS > - Add additionalProperties: false in DT bindings > > Changes since v1: > - Updated licensing info, "clocks" property and example in DT bindings > - Updated name of DT bindings document to match compatible string > - Removed 1 patch for addition of new sysfs attribute "count" > - Added support for COMPILE_TEST in Kconfig > - Updated naming of defines and regmap attribute > - Updated calculation of waveform high time and low time > - Added range checking for waveform high/low time > - Implemented .get_state > - Removed register writes for lead-in and count values (left to default) > - Updated register access to single-access > - Folded keembay_pwm_enable/disable_channel, > keembay_pwm_config_period/duty_cycle, > and keembay_pwm_config into keembay_pwm_apply > - Updated error messages/error codes > - Removed pwm_disable from keembay_pwm_remove > - Removed clk_prepare/clk_enable/clk_disable from driver > > Lai, Poey Seng (1): > pwm: Add PWM driver for Intel Keem Bay > > Vineetha G. Jaya Kumaran (1): > dt-bindings: pwm: keembay: Add bindings for Intel Keem Bay PWM > > .../bindings/pwm/intel,keembay-pwm.yaml | 47 ++++ > drivers/pwm/Kconfig | 9 + > drivers/pwm/Makefile | 1 + > drivers/pwm/pwm-keembay.c | 240 ++++++++++++++++++ > 4 files changed, 297 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pwm/intel,keembay-pwm.yaml > create mode 100644 drivers/pwm/pwm-keembay.c Both patches applied, though I did change the ordering so that checkpatch doesn't complain about the compatible string not being defined. I also changed the -ENOSYS error code to -EINVAL for the unsupported polarity case because checkpatch doesn't like it when we use -ENOSYS other than for system calls. I'm also going to follow up with a patch that makes the return value more consistent for these cases in other drivers. Thierry
Hi Thierry, Thanks for reviewing this patch. > From: Thierry Reding <thierry.reding@gmail.com> > Sent: Thursday, 12 November, 2020 4:22 AM > To: Ayyathurai, Vijayakannan <vijayakannan.ayyathurai@intel.com> > Cc: u.kleine-koenig@pengutronix.de; robh+dt@kernel.org; linux- > pwm@vger.kernel.org; devicetree@vger.kernel.org; Wan Mohamad, Wan > Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>; > andriy.shevchenko@linux.intel.com; mgross@linux.intel.com; Raja > Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com> > Subject: Re: [PATCH v14 0/2] Add PWM support for Intel Keem Bay SoC > > On Thu, Oct 22, 2020 at 03:14:45PM +0800, > vijayakannan.ayyathurai@intel.com wrote: > > From: Vijayakannan Ayyathurai <vijayakannan.ayyathurai@intel.com> > > > > Hi, > > > > This patch set enables the support for PWM in the Intel Keem Bay SoC. > > Keem Bay is an ARM based SoC, and the GPIO module allows > > configuration of 6 PWM outputs. > > > > Patch 1 adds the PWM driver and Patch 2 is for the required > > Device Tree bindings documentation. > > > > This driver was tested on the Keem Bay evaluation module board. > > > > Thank you. > > > > Regards, > > Vijay > > > > Changes since v13: > > - Fix indentation error in dt-binding. > > - Add maintainer name in dt-binding. > > > > Changes since v12: > > - Use devm_add_action_or_reset() as per Andy suggestion. > > - Do the clk_prepare_enable() after all devm_ stuff as per Uwe suggestion. > > - Optimize keembay_pwm_remove function. > > - Simplify the error handling for pwmchip_add. > > - In Kconfig, Use depends on ARCH_KEEMBAY || (ARM64 && > COMPILE_TEST) > > - Fix indentation error in dt-binding. > > - Add Uwe's Reviewed-by tag and Vijay's Co-developed-by tag. > > > > Changes since v11: > > - Minor variable name change to match the api needs. > > - Setup polarity as PWM_POLARITY_NORMAL. > > - Add comment for LEADIN register usage. > > > > Changes since v10: > > - Update low time calculation formula as per Uwe. > > - During distruct remove pwmchip first then disable the clock. > > > > Changes since v9: > > - Remove Reported-by tag from the commit log. > > > > Changes since v8: > > - Fix the compilation error reported by kernel test robot. > > - Add the tag Reported-by: kernel test robot <lkp@intel.com> > > - Minor correction in the pwm low time calculation formula. > > - Rebase with 5.9-rc7 > > > > Changes since v7: > > - Change the dependency as ARCH_KEEMBAY instead of ARM64 in Kconfig. > > - Use DIV_ROUND_DOWN_ULL instead of DIV_ROUND_CLOSEST_ULL. > > - Update the right formula as per Uwe. > > - List the tags in chronological order. > > - Add clk_disable_unprepare in the error paths. > > > > Changes since v6: > > - Add reviewed-by tag > > > > Changes since v5: > > - Reorder symbols/Kconfig in drivers/pwm/Kconfig and > drivers/pwm/Makefile > > - Use "Limitations" for consistency > > - Add clk_prepare_enable() > > - Reorder keembay_pwm_get_state() function call > > - Rework if conditional for channel disablement in .apply() > > - Remove channel disabling from .probe(), and clear LEADIN register bits > > in .apply instead > > - Update commit message for Patch 1 > > > > Changes since v4: > > - Add co-developed-by tag > > - Include mod_devicetable.h and remove of.h > > - Update comment with correct calulation for high/low time > > - Fix missing return from dev_err_probe > > > > Changes since v3: > > - Removed variable for address and calculate in place instead > > - Utilized u32_replace_bits() when updating KMB_PWM_LEADIN_OFFSET > > - Utilized dev_err_probe() for error reporting > > - Updated comments to use physical units > > - Updated error check for pwmchip_add() > > > > Changes since v2: > > - Include documentation about HW limitation/behaviour > > - Use hex values for KMB_PWM_COUNT_MAX > > - Redefine register macros > > - Utilize FIELD_GET/FIELD_PREP for calculating pwm_l/h_count and > > pwm_count > > - Round up duty cycle/period values > > - Get current hardware state in .apply instead of cached values > > - Do a polarity check before .enabled > > - Round high time/low time to closest value > > - Set enable bit in KMB_PWM_LEADIN_OFFSET to 0 in probe > > - Correct the naming for MODULE_ALIAS > > - Add additionalProperties: false in DT bindings > > > > Changes since v1: > > - Updated licensing info, "clocks" property and example in DT bindings > > - Updated name of DT bindings document to match compatible string > > - Removed 1 patch for addition of new sysfs attribute "count" > > - Added support for COMPILE_TEST in Kconfig > > - Updated naming of defines and regmap attribute > > - Updated calculation of waveform high time and low time > > - Added range checking for waveform high/low time > > - Implemented .get_state > > - Removed register writes for lead-in and count values (left to default) > > - Updated register access to single-access > > - Folded keembay_pwm_enable/disable_channel, > > keembay_pwm_config_period/duty_cycle, > > and keembay_pwm_config into keembay_pwm_apply > > - Updated error messages/error codes > > - Removed pwm_disable from keembay_pwm_remove > > - Removed clk_prepare/clk_enable/clk_disable from driver > > > > Lai, Poey Seng (1): > > pwm: Add PWM driver for Intel Keem Bay > > > > Vineetha G. Jaya Kumaran (1): > > dt-bindings: pwm: keembay: Add bindings for Intel Keem Bay PWM > > > > .../bindings/pwm/intel,keembay-pwm.yaml | 47 ++++ > > drivers/pwm/Kconfig | 9 + > > drivers/pwm/Makefile | 1 + > > drivers/pwm/pwm-keembay.c | 240 ++++++++++++++++++ > > 4 files changed, 297 insertions(+) > > create mode 100644 > Documentation/devicetree/bindings/pwm/intel,keembay-pwm.yaml > > create mode 100644 drivers/pwm/pwm-keembay.c > > Both patches applied, though I did change the ordering so that > checkpatch doesn't complain about the compatible string not being > defined. I also changed the -ENOSYS error code to -EINVAL for the > unsupported polarity case because checkpatch doesn't like it when > we use -ENOSYS other than for system calls. > > I'm also going to follow up with a patch that makes the return value > more consistent for these cases in other drivers. Thank you for your valuable time in fixing that. > > Thierry Thanks, Vijay
From: Vijayakannan Ayyathurai <vijayakannan.ayyathurai@intel.com> Hi, This patch set enables the support for PWM in the Intel Keem Bay SoC. Keem Bay is an ARM based SoC, and the GPIO module allows configuration of 6 PWM outputs. Patch 1 adds the PWM driver and Patch 2 is for the required Device Tree bindings documentation. This driver was tested on the Keem Bay evaluation module board. Thank you. Regards, Vijay Changes since v13: - Fix indentation error in dt-binding. - Add maintainer name in dt-binding. Changes since v12: - Use devm_add_action_or_reset() as per Andy suggestion. - Do the clk_prepare_enable() after all devm_ stuff as per Uwe suggestion. - Optimize keembay_pwm_remove function. - Simplify the error handling for pwmchip_add. - In Kconfig, Use depends on ARCH_KEEMBAY || (ARM64 && COMPILE_TEST) - Fix indentation error in dt-binding. - Add Uwe's Reviewed-by tag and Vijay's Co-developed-by tag. Changes since v11: - Minor variable name change to match the api needs. - Setup polarity as PWM_POLARITY_NORMAL. - Add comment for LEADIN register usage. Changes since v10: - Update low time calculation formula as per Uwe. - During distruct remove pwmchip first then disable the clock. Changes since v9: - Remove Reported-by tag from the commit log. Changes since v8: - Fix the compilation error reported by kernel test robot. - Add the tag Reported-by: kernel test robot <lkp@intel.com> - Minor correction in the pwm low time calculation formula. - Rebase with 5.9-rc7 Changes since v7: - Change the dependency as ARCH_KEEMBAY instead of ARM64 in Kconfig. - Use DIV_ROUND_DOWN_ULL instead of DIV_ROUND_CLOSEST_ULL. - Update the right formula as per Uwe. - List the tags in chronological order. - Add clk_disable_unprepare in the error paths. Changes since v6: - Add reviewed-by tag Changes since v5: - Reorder symbols/Kconfig in drivers/pwm/Kconfig and drivers/pwm/Makefile - Use "Limitations" for consistency - Add clk_prepare_enable() - Reorder keembay_pwm_get_state() function call - Rework if conditional for channel disablement in .apply() - Remove channel disabling from .probe(), and clear LEADIN register bits in .apply instead - Update commit message for Patch 1 Changes since v4: - Add co-developed-by tag - Include mod_devicetable.h and remove of.h - Update comment with correct calulation for high/low time - Fix missing return from dev_err_probe Changes since v3: - Removed variable for address and calculate in place instead - Utilized u32_replace_bits() when updating KMB_PWM_LEADIN_OFFSET - Utilized dev_err_probe() for error reporting - Updated comments to use physical units - Updated error check for pwmchip_add() Changes since v2: - Include documentation about HW limitation/behaviour - Use hex values for KMB_PWM_COUNT_MAX - Redefine register macros - Utilize FIELD_GET/FIELD_PREP for calculating pwm_l/h_count and pwm_count - Round up duty cycle/period values - Get current hardware state in .apply instead of cached values - Do a polarity check before .enabled - Round high time/low time to closest value - Set enable bit in KMB_PWM_LEADIN_OFFSET to 0 in probe - Correct the naming for MODULE_ALIAS - Add additionalProperties: false in DT bindings Changes since v1: - Updated licensing info, "clocks" property and example in DT bindings - Updated name of DT bindings document to match compatible string - Removed 1 patch for addition of new sysfs attribute "count" - Added support for COMPILE_TEST in Kconfig - Updated naming of defines and regmap attribute - Updated calculation of waveform high time and low time - Added range checking for waveform high/low time - Implemented .get_state - Removed register writes for lead-in and count values (left to default) - Updated register access to single-access - Folded keembay_pwm_enable/disable_channel, keembay_pwm_config_period/duty_cycle, and keembay_pwm_config into keembay_pwm_apply - Updated error messages/error codes - Removed pwm_disable from keembay_pwm_remove - Removed clk_prepare/clk_enable/clk_disable from driver Lai, Poey Seng (1): pwm: Add PWM driver for Intel Keem Bay Vineetha G. Jaya Kumaran (1): dt-bindings: pwm: keembay: Add bindings for Intel Keem Bay PWM .../bindings/pwm/intel,keembay-pwm.yaml | 47 ++++ drivers/pwm/Kconfig | 9 + drivers/pwm/Makefile | 1 + drivers/pwm/pwm-keembay.c | 240 ++++++++++++++++++ 4 files changed, 297 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/intel,keembay-pwm.yaml create mode 100644 drivers/pwm/pwm-keembay.c base-commit: 549738f15da0e5a00275977623be199fbbf7df50 prerequisite-patch-id: 0a348762b660d0d817b8e70cc71647e83173c78c prerequisite-patch-id: 0c6072cfe492b078c44ec864b8f9d1c76eada93b prerequisite-patch-id: 12b93428ee51a3d92ca973b928c0e0989f5d585e -- 2.17.1