2017-12-12 06:16:14

by Keerthy

[permalink] [raw]
Subject: [PATCH v5 0/8] omap: dmtimer: Move driver out of plat-omap

The series moves dmtimer out of plat-omap to drivers/clocksource.
The series also does a bunch of changes to pwm-omap-dmtimer code
to adapt to the driver migration and clean up plat specific
pdata-quirks and use the dmtimer platform data.

Boot tested on DRA7-EVM and AM437X-GP-EVM.
Compile tested omap1_defconfig.

This is based on top of linux-next branch.

Changes from v4:

* Made OMAP_DM_TIMER config option silent.
* Changed the driver name to timer-dm.c

Changes from v3:

* Reverted to v2 approach of using dev_get_platdata to fetch dmtimer ops.

Changes from V2:

* Wrapped the inline functions in header file under OMAP2PLUS
* Added a new of helper function to fetch plat_data from of node.

Keerthy (8):
clocksource: dmtimer: Remove all the exports
arm: omap: timer: Wrap the inline functions under OMAP2PLUS define
arm: omap: Move dmtimer.h out of plat-omap
arm: OMAP: Move dmtimer driver out of plat-omap to drivers under
clocksource
dmtimer: Add timer ops to the platform data structure
clocksource: dmtimer: Populate the timer ops to the pdata
pwm: pwm-omap-dmtimer: Adapt driver to utilize dmtimer pdata ops
arm: omap: pdata-quirks: Remove unused timer pdata

arch/arm/mach-omap1/pm.c | 2 +-
arch/arm/mach-omap1/timer.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_2420_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_2430_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_54xx_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_81xx_data.c | 2 +-
arch/arm/mach-omap2/pdata-quirks.c | 32 -------------
arch/arm/mach-omap2/timer.c | 2 +-
arch/arm/plat-omap/Kconfig | 6 ---
arch/arm/plat-omap/Makefile | 1 -
drivers/clocksource/Kconfig | 3 ++
drivers/clocksource/Makefile | 1 +
.../dmtimer.c => drivers/clocksource/timer-dm.c | 54 +++++++++++-----------
drivers/pwm/pwm-omap-dmtimer.c | 39 +++++++++-------
.../include/plat => include/clocksource}/dmtimer.h | 8 +++-
include/linux/platform_data/dmtimer-omap.h | 38 +++++++++++++++
20 files changed, 108 insertions(+), 96 deletions(-)
rename arch/arm/plat-omap/dmtimer.c => drivers/clocksource/timer-dm.c (95%)
rename {arch/arm/plat-omap/include/plat => include/clocksource}/dmtimer.h (97%)

--
1.9.1


2017-12-12 06:14:42

by Keerthy

[permalink] [raw]
Subject: [PATCH v5 1/8] clocksource: dmtimer: Remove all the exports

Remove all the unwanted exports from the driver

Signed-off-by: Keerthy <[email protected]>
Reviewed-by: Sebastian Reichel <[email protected]>
---
Changes in v3:

* Added Sebastian's Reviewed-by.

Changes in v2:

* No code changes in this v2 version. Only enhanced patch
statistics for renames.

arch/arm/plat-omap/dmtimer.c | 27 ---------------------------
1 file changed, 27 deletions(-)

diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index d443e48..72565fc 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -302,7 +302,6 @@ struct omap_dm_timer *omap_dm_timer_request(void)
{
return _omap_dm_timer_request(REQUEST_ANY, NULL);
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_request);

struct omap_dm_timer *omap_dm_timer_request_specific(int id)
{
@@ -315,7 +314,6 @@ struct omap_dm_timer *omap_dm_timer_request_specific(int id)

return _omap_dm_timer_request(REQUEST_BY_ID, &id);
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_request_specific);

/**
* omap_dm_timer_request_by_cap - Request a timer by capability
@@ -330,7 +328,6 @@ struct omap_dm_timer *omap_dm_timer_request_by_cap(u32 cap)
{
return _omap_dm_timer_request(REQUEST_BY_CAP, &cap);
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_request_by_cap);

/**
* omap_dm_timer_request_by_node - Request a timer by device-tree node
@@ -346,7 +343,6 @@ struct omap_dm_timer *omap_dm_timer_request_by_node(struct device_node *np)

return _omap_dm_timer_request(REQUEST_BY_NODE, np);
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_request_by_node);

int omap_dm_timer_free(struct omap_dm_timer *timer)
{
@@ -359,7 +355,6 @@ int omap_dm_timer_free(struct omap_dm_timer *timer)
timer->reserved = 0;
return 0;
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_free);

void omap_dm_timer_enable(struct omap_dm_timer *timer)
{
@@ -379,13 +374,11 @@ void omap_dm_timer_enable(struct omap_dm_timer *timer)
}
}
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_enable);

void omap_dm_timer_disable(struct omap_dm_timer *timer)
{
pm_runtime_put_sync(&timer->pdev->dev);
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_disable);

int omap_dm_timer_get_irq(struct omap_dm_timer *timer)
{
@@ -393,7 +386,6 @@ int omap_dm_timer_get_irq(struct omap_dm_timer *timer)
return timer->irq;
return -EINVAL;
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_get_irq);

#if defined(CONFIG_ARCH_OMAP1)
#include <mach/hardware.h>
@@ -429,7 +421,6 @@ __u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)

return inputmask;
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_modify_idlect_mask);

#else

@@ -439,7 +430,6 @@ struct clk *omap_dm_timer_get_fclk(struct omap_dm_timer *timer)
return timer->fclk;
return NULL;
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_get_fclk);

__u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)
{
@@ -447,7 +437,6 @@ __u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)

return 0;
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_modify_idlect_mask);

#endif

@@ -461,7 +450,6 @@ int omap_dm_timer_trigger(struct omap_dm_timer *timer)
omap_dm_timer_write_reg(timer, OMAP_TIMER_TRIGGER_REG, 0);
return 0;
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_trigger);

int omap_dm_timer_start(struct omap_dm_timer *timer)
{
@@ -482,7 +470,6 @@ int omap_dm_timer_start(struct omap_dm_timer *timer)
timer->context.tclr = l;
return 0;
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_start);

int omap_dm_timer_stop(struct omap_dm_timer *timer)
{
@@ -506,7 +493,6 @@ int omap_dm_timer_stop(struct omap_dm_timer *timer)
omap_dm_timer_disable(timer);
return 0;
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_stop);

int omap_dm_timer_set_source(struct omap_dm_timer *timer, int source)
{
@@ -569,7 +555,6 @@ int omap_dm_timer_set_source(struct omap_dm_timer *timer, int source)

return ret;
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_set_source);

int omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload,
unsigned int load)
@@ -595,7 +580,6 @@ int omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload,
omap_dm_timer_disable(timer);
return 0;
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_set_load);

/* Optimized set_load which removes costly spin wait in timer_start */
int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, int autoreload,
@@ -625,7 +609,6 @@ int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, int autoreload,
timer->context.tcrr = load;
return 0;
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_set_load_start);

int omap_dm_timer_set_match(struct omap_dm_timer *timer, int enable,
unsigned int match)
@@ -650,7 +633,6 @@ int omap_dm_timer_set_match(struct omap_dm_timer *timer, int enable,
omap_dm_timer_disable(timer);
return 0;
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_set_match);

int omap_dm_timer_set_pwm(struct omap_dm_timer *timer, int def_on,
int toggle, int trigger)
@@ -676,7 +658,6 @@ int omap_dm_timer_set_pwm(struct omap_dm_timer *timer, int def_on,
omap_dm_timer_disable(timer);
return 0;
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_set_pwm);

int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer, int prescaler)
{
@@ -699,7 +680,6 @@ int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer, int prescaler)
omap_dm_timer_disable(timer);
return 0;
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_set_prescaler);

int omap_dm_timer_set_int_enable(struct omap_dm_timer *timer,
unsigned int value)
@@ -716,7 +696,6 @@ int omap_dm_timer_set_int_enable(struct omap_dm_timer *timer,
omap_dm_timer_disable(timer);
return 0;
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_set_int_enable);

/**
* omap_dm_timer_set_int_disable - disable timer interrupts
@@ -747,7 +726,6 @@ int omap_dm_timer_set_int_disable(struct omap_dm_timer *timer, u32 mask)
omap_dm_timer_disable(timer);
return 0;
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_set_int_disable);

unsigned int omap_dm_timer_read_status(struct omap_dm_timer *timer)
{
@@ -762,7 +740,6 @@ unsigned int omap_dm_timer_read_status(struct omap_dm_timer *timer)

return l;
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_read_status);

int omap_dm_timer_write_status(struct omap_dm_timer *timer, unsigned int value)
{
@@ -773,7 +750,6 @@ int omap_dm_timer_write_status(struct omap_dm_timer *timer, unsigned int value)

return 0;
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_write_status);

unsigned int omap_dm_timer_read_counter(struct omap_dm_timer *timer)
{
@@ -784,7 +760,6 @@ unsigned int omap_dm_timer_read_counter(struct omap_dm_timer *timer)

return __omap_dm_timer_read_counter(timer, timer->posted);
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_read_counter);

int omap_dm_timer_write_counter(struct omap_dm_timer *timer, unsigned int value)
{
@@ -799,7 +774,6 @@ int omap_dm_timer_write_counter(struct omap_dm_timer *timer, unsigned int value)
timer->context.tcrr = value;
return 0;
}
-EXPORT_SYMBOL_GPL(omap_dm_timer_write_counter);

int omap_dm_timers_active(void)
{
@@ -816,7 +790,6 @@ int omap_dm_timers_active(void)
}
return 0;
}
-EXPORT_SYMBOL_GPL(omap_dm_timers_active);

static const struct of_device_id omap_timer_match[];

--
1.9.1

2017-12-12 06:14:45

by Keerthy

[permalink] [raw]
Subject: [PATCH v5 2/8] arm: omap: timer: Wrap the inline functions under OMAP2PLUS define

Wrap the inline functions under OMAP2PLUS/OMAP1 defines.

Signed-off-by: Keerthy <[email protected]>
---
arch/arm/plat-omap/include/plat/dmtimer.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h b/arch/arm/plat-omap/include/plat/dmtimer.h
index dd79f30..862ad62 100644
--- a/arch/arm/plat-omap/include/plat/dmtimer.h
+++ b/arch/arm/plat-omap/include/plat/dmtimer.h
@@ -276,6 +276,12 @@ struct omap_dm_timer {
#define OMAP_TIMER_TICK_INT_MASK_COUNT_REG \
(_OMAP_TIMER_TICK_INT_MASK_COUNT_OFFSET | (WP_TOWR << WPSHIFT))

+/*
+ * The below are inlined to optimize code size for system timers. Other code
+ * should not need these at all, see
+ * include/linux/platform_data/pwm_omap_dmtimer.h
+ */
+#if defined(CONFIG_ARCH_OMAP1) || defined(CONFIG_ARCH_OMAP2PLUS)
static inline u32 __omap_dm_timer_read(struct omap_dm_timer *timer, u32 reg,
int posted)
{
@@ -414,5 +420,5 @@ static inline void __omap_dm_timer_write_status(struct omap_dm_timer *timer,
{
writel_relaxed(value, timer->irq_stat);
}
-
+#endif /* CONFIG_ARCH_OMAP1 || CONFIG_ARCH_OMAP2PLUS */
#endif /* __ASM_ARCH_DMTIMER_H */
--
1.9.1

2017-12-12 06:14:58

by Keerthy

[permalink] [raw]
Subject: [PATCH v5 4/8] arm: OMAP: Move dmtimer driver out of plat-omap to drivers under clocksource

Move the dmtimer driver out of plat-omap to clocksource.
So that non-omap devices also could use this.

No Code changes done to the driver file only renamed to timer-dm.c.
Also removed the config dependencies for OMAP_DM_TIMER.

Signed-off-by: Keerthy <[email protected]>
Reviewed-by: Sebastian Reichel <[email protected]>
---

Changes in v5:

* Made OMAP_DM_TIMER config option silent.
* Changed the driver name to timer-dm.c

Changes in v3:

* Added Sebastian's Reviewed-by.

Changes in v2:

* No code changes in this v2 version. Only enhanced patch
statistics for renames.
arch/arm/plat-omap/Kconfig | 6 ------
arch/arm/plat-omap/Makefile | 1 -
drivers/clocksource/Kconfig | 3 +++
drivers/clocksource/Makefile | 1 +
arch/arm/plat-omap/dmtimer.c => drivers/clocksource/timer-dm.c | 0
5 files changed, 4 insertions(+), 7 deletions(-)
rename arch/arm/plat-omap/dmtimer.c => drivers/clocksource/timer-dm.c (100%)

diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
index 7276afe..afc1a1d 100644
--- a/arch/arm/plat-omap/Kconfig
+++ b/arch/arm/plat-omap/Kconfig
@@ -106,12 +106,6 @@ config OMAP3_L2_AUX_SECURE_SERVICE_SET_ID
help
PPA routine service ID for setting L2 auxiliary control register.

-config OMAP_DM_TIMER
- bool "Use dual-mode timer"
- depends on ARCH_OMAP16XX || ARCH_OMAP2PLUS
- help
- Select this option if you want to use OMAP Dual-Mode timers.
-
config OMAP_SERIAL_WAKE
bool "Enable wake-up events for serial ports"
depends on ARCH_OMAP1 && OMAP_MUX
diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
index 47e1867..7215ada 100644
--- a/arch/arm/plat-omap/Makefile
+++ b/arch/arm/plat-omap/Makefile
@@ -9,5 +9,4 @@ obj-y := sram.o dma.o counter_32k.o

# omap_device support (OMAP2+ only at the moment)

-obj-$(CONFIG_OMAP_DM_TIMER) += dmtimer.o
obj-$(CONFIG_OMAP_DEBUG_LEDS) += debug-leds.o
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index c729a88..3f799b2 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -21,6 +21,9 @@ config CLKEVT_I8253
config I8253_LOCK
bool

+config OMAP_DM_TIMER
+ bool
+
config CLKBLD_I8253
def_bool y if CLKSRC_I8253 || CLKEVT_I8253 || I8253_LOCK

diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 72711f1..27b5497 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_EM_TIMER_STI) += em_sti.o
obj-$(CONFIG_CLKBLD_I8253) += i8253.o
obj-$(CONFIG_CLKSRC_MMIO) += mmio.o
obj-$(CONFIG_DIGICOLOR_TIMER) += timer-digicolor.o
+obj-$(CONFIG_OMAP_DM_TIMER) += timer-dm.o
obj-$(CONFIG_DW_APB_TIMER) += dw_apb_timer.o
obj-$(CONFIG_DW_APB_TIMER_OF) += dw_apb_timer_of.o
obj-$(CONFIG_FTTMR010_TIMER) += timer-fttmr010.o
diff --git a/arch/arm/plat-omap/dmtimer.c b/drivers/clocksource/timer-dm.c
similarity index 100%
rename from arch/arm/plat-omap/dmtimer.c
rename to drivers/clocksource/timer-dm.c
--
1.9.1

2017-12-12 06:15:07

by Keerthy

[permalink] [raw]
Subject: [PATCH v5 8/8] arm: omap: pdata-quirks: Remove unused timer pdata

Remove unused timer pdata.

Signed-off-by: Keerthy <[email protected]>
Reviewed-by: Sebastian Reichel <[email protected]>
---

Changes in v3:

* Added Sebastian's Reviewed-by.

Changes in v2:

* No code changes in this v2 version. Only enhanced patch
statistics for renames.

arch/arm/mach-omap2/pdata-quirks.c | 32 --------------------------------
1 file changed, 32 deletions(-)

diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
index ad9df86..e7d7fc7 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -24,10 +24,8 @@
#include <linux/platform_data/hsmmc-omap.h>
#include <linux/platform_data/iommu-omap.h>
#include <linux/platform_data/wkup_m3.h>
-#include <linux/platform_data/pwm_omap_dmtimer.h>
#include <linux/platform_data/media/ir-rx51.h>
#include <linux/platform_data/asoc-ti-mcbsp.h>
-#include <clocksource/dmtimer.h>

#include "common.h"
#include "common-board-devices.h"
@@ -477,33 +475,6 @@ void omap_auxdata_legacy_init(struct device *dev)
dev->platform_data = &twl_gpio_auxdata;
}

-/* Dual mode timer PWM callbacks platdata */
-#if IS_ENABLED(CONFIG_OMAP_DM_TIMER)
-static struct pwm_omap_dmtimer_pdata pwm_dmtimer_pdata = {
- .request_by_node = omap_dm_timer_request_by_node,
- .request_specific = omap_dm_timer_request_specific,
- .request = omap_dm_timer_request,
- .set_source = omap_dm_timer_set_source,
- .get_irq = omap_dm_timer_get_irq,
- .set_int_enable = omap_dm_timer_set_int_enable,
- .set_int_disable = omap_dm_timer_set_int_disable,
- .free = omap_dm_timer_free,
- .enable = omap_dm_timer_enable,
- .disable = omap_dm_timer_disable,
- .get_fclk = omap_dm_timer_get_fclk,
- .start = omap_dm_timer_start,
- .stop = omap_dm_timer_stop,
- .set_load = omap_dm_timer_set_load,
- .set_match = omap_dm_timer_set_match,
- .set_pwm = omap_dm_timer_set_pwm,
- .set_prescaler = omap_dm_timer_set_prescaler,
- .read_counter = omap_dm_timer_read_counter,
- .write_counter = omap_dm_timer_write_counter,
- .read_status = omap_dm_timer_read_status,
- .write_status = omap_dm_timer_write_status,
-};
-#endif
-
static struct ir_rx51_platform_data __maybe_unused rx51_ir_data = {
.set_max_mpu_wakeup_lat = omap_pm_set_max_mpu_wakeup_lat,
};
@@ -572,9 +543,6 @@ static void __init omap3_mcbsp_init(void) {}
OF_DEV_AUXDATA("ti,am4372-wkup-m3", 0x44d00000, "44d00000.wkup_m3",
&wkup_m3_data),
#endif
-#if IS_ENABLED(CONFIG_OMAP_DM_TIMER)
- OF_DEV_AUXDATA("ti,omap-dmtimer-pwm", 0, NULL, &pwm_dmtimer_pdata),
-#endif
#if defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_SOC_OMAP5)
OF_DEV_AUXDATA("ti,omap4-iommu", 0x4a066000, "4a066000.mmu",
&omap4_iommu_pdata),
--
1.9.1

2017-12-12 06:15:14

by Keerthy

[permalink] [raw]
Subject: [PATCH v5 6/8] clocksource: dmtimer: Populate the timer ops to the pdata

Add the timer ops to the platform data

Signed-off-by: Keerthy <[email protected]>
Reviewed-by: Sebastian Reichel <[email protected]>
---

Changes in v3:

* Added Sebastian's Reviewed-by.

Changes in v2:

* No code changes in this v2 version. Only enhanced patch
statistics for renames.

drivers/clocksource/timer-dm.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)

diff --git a/drivers/clocksource/timer-dm.c b/drivers/clocksource/timer-dm.c
index afe1dc9..1cbd954 100644
--- a/drivers/clocksource/timer-dm.c
+++ b/drivers/clocksource/timer-dm.c
@@ -922,8 +922,33 @@ static int omap_dm_timer_remove(struct platform_device *pdev)
return ret;
}

+static struct omap_dm_timer_ops dmtimer_ops = {
+ .request_by_node = omap_dm_timer_request_by_node,
+ .request_specific = omap_dm_timer_request_specific,
+ .request = omap_dm_timer_request,
+ .set_source = omap_dm_timer_set_source,
+ .get_irq = omap_dm_timer_get_irq,
+ .set_int_enable = omap_dm_timer_set_int_enable,
+ .set_int_disable = omap_dm_timer_set_int_disable,
+ .free = omap_dm_timer_free,
+ .enable = omap_dm_timer_enable,
+ .disable = omap_dm_timer_disable,
+ .get_fclk = omap_dm_timer_get_fclk,
+ .start = omap_dm_timer_start,
+ .stop = omap_dm_timer_stop,
+ .set_load = omap_dm_timer_set_load,
+ .set_match = omap_dm_timer_set_match,
+ .set_pwm = omap_dm_timer_set_pwm,
+ .set_prescaler = omap_dm_timer_set_prescaler,
+ .read_counter = omap_dm_timer_read_counter,
+ .write_counter = omap_dm_timer_write_counter,
+ .read_status = omap_dm_timer_read_status,
+ .write_status = omap_dm_timer_write_status,
+};
+
static const struct dmtimer_platform_data omap3plus_pdata = {
.timer_errata = OMAP_TIMER_ERRATA_I103_I767,
+ .timer_ops = &dmtimer_ops,
};

static const struct of_device_id omap_timer_match[] = {
--
1.9.1

2017-12-12 06:15:37

by Keerthy

[permalink] [raw]
Subject: [PATCH v5 7/8] pwm: pwm-omap-dmtimer: Adapt driver to utilize dmtimer pdata ops

Adapt driver to utilize dmtimer pdata ops instead of pdata-quirks.

Signed-off-by: Keerthy <[email protected]>
---

Changes in v4:

* Switched to dev_get_platdata.

Changes in v3:

* Used of_find_platdata_by_node function to fetch platform
data for timer node.

drivers/pwm/pwm-omap-dmtimer.c | 39 ++++++++++++++++++++++-----------------
1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/drivers/pwm/pwm-omap-dmtimer.c b/drivers/pwm/pwm-omap-dmtimer.c
index 5ad42f3..3b27aff 100644
--- a/drivers/pwm/pwm-omap-dmtimer.c
+++ b/drivers/pwm/pwm-omap-dmtimer.c
@@ -23,6 +23,7 @@
#include <linux/mutex.h>
#include <linux/of.h>
#include <linux/of_platform.h>
+#include <linux/platform_data/dmtimer-omap.h>
#include <linux/platform_data/pwm_omap_dmtimer.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
@@ -37,7 +38,7 @@ struct pwm_omap_dmtimer_chip {
struct pwm_chip chip;
struct mutex mutex;
pwm_omap_dmtimer *dm_timer;
- struct pwm_omap_dmtimer_pdata *pdata;
+ struct omap_dm_timer_ops *pdata;
struct platform_device *dm_timer_pdev;
};

@@ -242,19 +243,33 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
struct device_node *timer;
+ struct platform_device *timer_pdev;
struct pwm_omap_dmtimer_chip *omap;
- struct pwm_omap_dmtimer_pdata *pdata;
+ struct dmtimer_platform_data *timer_pdata;
+ struct omap_dm_timer_ops *pdata;
pwm_omap_dmtimer *dm_timer;
u32 v;
int status;

- pdata = dev_get_platdata(&pdev->dev);
- if (!pdata) {
- dev_err(&pdev->dev, "Missing dmtimer platform data\n");
+ timer = of_parse_phandle(np, "ti,timers", 0);
+ if (!timer)
+ return -ENODEV;
+
+ timer_pdev = of_find_device_by_node(timer);
+ if (!timer_pdev) {
+ dev_err(&pdev->dev, "Unable to find Timer pdev\n");
+ return -ENODEV;
+ }
+
+ timer_pdata = dev_get_platdata(&timer_pdev->dev);
+ if (!timer_pdata) {
+ dev_err(&pdev->dev, "dmtimer pdata structure NULL\n");
return -EINVAL;
}

- if (!pdata->request_by_node ||
+ pdata = timer_pdata->timer_ops;
+
+ if (!pdata || !pdata->request_by_node ||
!pdata->free ||
!pdata->enable ||
!pdata->disable ||
@@ -270,10 +285,6 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev)
return -EINVAL;
}

- timer = of_parse_phandle(np, "ti,timers", 0);
- if (!timer)
- return -ENODEV;
-
if (!of_get_property(timer, "ti,timer-pwm", NULL)) {
dev_err(&pdev->dev, "Missing ti,timer-pwm capability\n");
return -ENODEV;
@@ -291,13 +302,7 @@ static int pwm_omap_dmtimer_probe(struct platform_device *pdev)

omap->pdata = pdata;
omap->dm_timer = dm_timer;
-
- omap->dm_timer_pdev = of_find_device_by_node(timer);
- if (!omap->dm_timer_pdev) {
- dev_err(&pdev->dev, "Unable to find timer pdev\n");
- omap->pdata->free(dm_timer);
- return -EINVAL;
- }
+ omap->dm_timer_pdev = timer_pdev;

/*
* Ensure that the timer is stopped before we allow PWM core to call
--
1.9.1

2017-12-12 06:14:40

by Keerthy

[permalink] [raw]
Subject: [PATCH v5 3/8] arm: omap: Move dmtimer.h out of plat-omap

The header file is currently under plat-omap directory
under arch/omap. Move this out to an accessible place.

No Code changes done to the header file.

Signed-off-by: Keerthy <[email protected]>
Reviewed-by: Sebastian Reichel <[email protected]>
---
Changes in v3:

* Added Sebastian's Reviewed-by.

Changes in v2:

* No code changes in this v2 version. Only enhanced patch
statistics for renames.

arch/arm/mach-omap1/pm.c | 2 +-
arch/arm/mach-omap1/timer.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_2420_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_2430_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_54xx_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_81xx_data.c | 2 +-
arch/arm/mach-omap2/pdata-quirks.c | 2 +-
arch/arm/mach-omap2/timer.c | 2 +-
arch/arm/plat-omap/dmtimer.c | 2 +-
{arch/arm/plat-omap/include/plat => include/clocksource}/dmtimer.h | 0
14 files changed, 13 insertions(+), 13 deletions(-)
rename {arch/arm/plat-omap/include/plat => include/clocksource}/dmtimer.h (100%)

diff --git a/arch/arm/mach-omap1/pm.c b/arch/arm/mach-omap1/pm.c
index f1135bf..a07d47cf 100644
--- a/arch/arm/mach-omap1/pm.c
+++ b/arch/arm/mach-omap1/pm.c
@@ -55,7 +55,7 @@
#include <mach/tc.h>
#include <mach/mux.h>
#include <linux/omap-dma.h>
-#include <plat/dmtimer.h>
+#include <clocksource/dmtimer.h>

#include <mach/irqs.h>

diff --git a/arch/arm/mach-omap1/timer.c b/arch/arm/mach-omap1/timer.c
index 8fb1ec6..7c057ab 100644
--- a/arch/arm/mach-omap1/timer.c
+++ b/arch/arm/mach-omap1/timer.c
@@ -27,7 +27,7 @@
#include <linux/platform_device.h>
#include <linux/platform_data/dmtimer-omap.h>

-#include <plat/dmtimer.h>
+#include <clocksource/dmtimer.h>

#include "soc.h"

diff --git a/arch/arm/mach-omap2/omap_hwmod_2420_data.c b/arch/arm/mach-omap2/omap_hwmod_2420_data.c
index 1a15a34..45c1043 100644
--- a/arch/arm/mach-omap2/omap_hwmod_2420_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_2420_data.c
@@ -16,7 +16,7 @@
#include <linux/i2c-omap.h>
#include <linux/platform_data/spi-omap2-mcspi.h>
#include <linux/omap-dma.h>
-#include <plat/dmtimer.h>
+#include <clocksource/dmtimer.h>

#include "omap_hwmod.h"
#include "l3_2xxx.h"
diff --git a/arch/arm/mach-omap2/omap_hwmod_2430_data.c b/arch/arm/mach-omap2/omap_hwmod_2430_data.c
index 3801850..892ca58 100644
--- a/arch/arm/mach-omap2/omap_hwmod_2430_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_2430_data.c
@@ -18,7 +18,7 @@
#include <linux/platform_data/hsmmc-omap.h>
#include <linux/platform_data/spi-omap2-mcspi.h>
#include <linux/omap-dma.h>
-#include <plat/dmtimer.h>
+#include <clocksource/dmtimer.h>

#include "omap_hwmod.h"
#include "l3_2xxx.h"
diff --git a/arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c b/arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c
index beec4cd..82b51c0 100644
--- a/arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c
@@ -11,7 +11,7 @@

#include <linux/platform_data/gpio-omap.h>
#include <linux/omap-dma.h>
-#include <plat/dmtimer.h>
+#include <clocksource/dmtimer.h>
#include <linux/platform_data/spi-omap2-mcspi.h>

#include "omap_hwmod.h"
diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 52c9d58..310aef5 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -25,7 +25,7 @@
#include "l4_3xxx.h"
#include <linux/platform_data/asoc-ti-mcbsp.h>
#include <linux/platform_data/spi-omap2-mcspi.h>
-#include <plat/dmtimer.h>
+#include <clocksource/dmtimer.h>

#include "soc.h"
#include "omap_hwmod.h"
diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index c477096..22e0e38 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -30,7 +30,7 @@

#include <linux/platform_data/spi-omap2-mcspi.h>
#include <linux/platform_data/asoc-ti-mcbsp.h>
-#include <plat/dmtimer.h>
+#include <clocksource/dmtimer.h>

#include "omap_hwmod.h"
#include "omap_hwmod_common_data.h"
diff --git a/arch/arm/mach-omap2/omap_hwmod_54xx_data.c b/arch/arm/mach-omap2/omap_hwmod_54xx_data.c
index 988e7ea..530334e 100644
--- a/arch/arm/mach-omap2/omap_hwmod_54xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_54xx_data.c
@@ -26,7 +26,7 @@
#include <linux/omap-dma.h>
#include <linux/platform_data/spi-omap2-mcspi.h>
#include <linux/platform_data/asoc-ti-mcbsp.h>
-#include <plat/dmtimer.h>
+#include <clocksource/dmtimer.h>

#include "omap_hwmod.h"
#include "omap_hwmod_common_data.h"
diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
index d05e553d..adabdef 100644
--- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
@@ -26,7 +26,7 @@
#include <linux/omap-dma.h>
#include <linux/platform_data/spi-omap2-mcspi.h>
#include <linux/platform_data/asoc-ti-mcbsp.h>
-#include <plat/dmtimer.h>
+#include <clocksource/dmtimer.h>

#include "omap_hwmod.h"
#include "omap_hwmod_common_data.h"
diff --git a/arch/arm/mach-omap2/omap_hwmod_81xx_data.c b/arch/arm/mach-omap2/omap_hwmod_81xx_data.c
index 77a515b..d05dd2d 100644
--- a/arch/arm/mach-omap2/omap_hwmod_81xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_81xx_data.c
@@ -18,7 +18,7 @@
#include <linux/platform_data/gpio-omap.h>
#include <linux/platform_data/hsmmc-omap.h>
#include <linux/platform_data/spi-omap2-mcspi.h>
-#include <plat/dmtimer.h>
+#include <clocksource/dmtimer.h>

#include "omap_hwmod_common_data.h"
#include "cm81xx.h"
diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
index 6b433fc..ad9df86 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -27,7 +27,7 @@
#include <linux/platform_data/pwm_omap_dmtimer.h>
#include <linux/platform_data/media/ir-rx51.h>
#include <linux/platform_data/asoc-ti-mcbsp.h>
-#include <plat/dmtimer.h>
+#include <clocksource/dmtimer.h>

#include "common.h"
#include "common-board-devices.h"
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index ece09c9..31c1b01 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -26,6 +26,7 @@
* License. See the file "COPYING" in the main directory of this archive
* for more details.
*/
+#include <clocksource/dmtimer.h>
#include <linux/init.h>
#include <linux/time.h>
#include <linux/interrupt.h>
@@ -49,7 +50,6 @@
#include "omap_hwmod.h"
#include "omap_device.h"
#include <plat/counter-32k.h>
-#include <plat/dmtimer.h>
#include "omap-pm.h"

#include "soc.h"
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 72565fc..afe1dc9 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -47,7 +47,7 @@
#include <linux/platform_device.h>
#include <linux/platform_data/dmtimer-omap.h>

-#include <plat/dmtimer.h>
+#include <clocksource/dmtimer.h>

static u32 omap_reserved_systimers;
static LIST_HEAD(omap_timer_list);
diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h b/include/clocksource/dmtimer.h
similarity index 100%
rename from arch/arm/plat-omap/include/plat/dmtimer.h
rename to include/clocksource/dmtimer.h
--
1.9.1

2017-12-12 06:16:09

by Keerthy

[permalink] [raw]
Subject: [PATCH v5 5/8] dmtimer: Add timer ops to the platform data structure

Add timer ops to the platform data structure

Signed-off-by: Keerthy <[email protected]>
Reviewed-by: Sebastian Reichel <[email protected]>
---

Changes in v3:

* Added Sebastian's Reviewed-by.

Changes in v2:

* No code changes in this v2 version. Only enhanced patch
statistics for renames.

include/linux/platform_data/dmtimer-omap.h | 38 ++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)

diff --git a/include/linux/platform_data/dmtimer-omap.h b/include/linux/platform_data/dmtimer-omap.h
index a19b78d..a3e1794 100644
--- a/include/linux/platform_data/dmtimer-omap.h
+++ b/include/linux/platform_data/dmtimer-omap.h
@@ -20,12 +20,50 @@
#ifndef __PLATFORM_DATA_DMTIMER_OMAP_H__
#define __PLATFORM_DATA_DMTIMER_OMAP_H__

+struct omap_dm_timer_ops {
+ struct omap_dm_timer *(*request_by_node)(struct device_node *np);
+ struct omap_dm_timer *(*request_specific)(int timer_id);
+ struct omap_dm_timer *(*request)(void);
+
+ int (*free)(struct omap_dm_timer *timer);
+
+ void (*enable)(struct omap_dm_timer *timer);
+ void (*disable)(struct omap_dm_timer *timer);
+
+ int (*get_irq)(struct omap_dm_timer *timer);
+ int (*set_int_enable)(struct omap_dm_timer *timer,
+ unsigned int value);
+ int (*set_int_disable)(struct omap_dm_timer *timer, u32 mask);
+
+ struct clk *(*get_fclk)(struct omap_dm_timer *timer);
+
+ int (*start)(struct omap_dm_timer *timer);
+ int (*stop)(struct omap_dm_timer *timer);
+ int (*set_source)(struct omap_dm_timer *timer, int source);
+
+ int (*set_load)(struct omap_dm_timer *timer, int autoreload,
+ unsigned int value);
+ int (*set_match)(struct omap_dm_timer *timer, int enable,
+ unsigned int match);
+ int (*set_pwm)(struct omap_dm_timer *timer, int def_on,
+ int toggle, int trigger);
+ int (*set_prescaler)(struct omap_dm_timer *timer, int prescaler);
+
+ unsigned int (*read_counter)(struct omap_dm_timer *timer);
+ int (*write_counter)(struct omap_dm_timer *timer,
+ unsigned int value);
+ unsigned int (*read_status)(struct omap_dm_timer *timer);
+ int (*write_status)(struct omap_dm_timer *timer,
+ unsigned int value);
+};
+
struct dmtimer_platform_data {
/* set_timer_src - Only used for OMAP1 devices */
int (*set_timer_src)(struct platform_device *pdev, int source);
u32 timer_capability;
u32 timer_errata;
int (*get_context_loss_count)(struct device *);
+ struct omap_dm_timer_ops *timer_ops;
};

#endif /* __PLATFORM_DATA_DMTIMER_OMAP_H__ */
--
1.9.1

2017-12-12 07:16:30

by Ladislav Michl

[permalink] [raw]
Subject: Re: [PATCH v5 1/8] clocksource: dmtimer: Remove all the exports

Keerthy,

On Tue, Dec 12, 2017 at 11:42:10AM +0530, Keerthy wrote:
> Remove all the unwanted exports from the driver

I'm adding event capture capability to the pwm-omap driver and so far used
v4.15-rc3 as codebase.

Intended use is an IR receiver; for that I need to measure pulses width and
spaces between pulses. So DM timer was setup to generate interupt after
both TCAR1 and TCAR2 are filled, values are passed to IR decoder and
TCAR_IT_FLAG is cleared.

Of course, this is just proof of concept and needs to be polished and
generalized, but to make it at least work I need functions you just
unexported (plus some new).

Question is whenever we need this level of indirection (omap_dm_timer_ops)
or plain exports are enough.

Thank you,
ladis

> Signed-off-by: Keerthy <[email protected]>
> Reviewed-by: Sebastian Reichel <[email protected]>
> ---
> Changes in v3:
>
> * Added Sebastian's Reviewed-by.
>
> Changes in v2:
>
> * No code changes in this v2 version. Only enhanced patch
> statistics for renames.
>
> arch/arm/plat-omap/dmtimer.c | 27 ---------------------------
> 1 file changed, 27 deletions(-)
>
> diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
> index d443e48..72565fc 100644
> --- a/arch/arm/plat-omap/dmtimer.c
> +++ b/arch/arm/plat-omap/dmtimer.c
> @@ -302,7 +302,6 @@ struct omap_dm_timer *omap_dm_timer_request(void)
> {
> return _omap_dm_timer_request(REQUEST_ANY, NULL);
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_request);
>
> struct omap_dm_timer *omap_dm_timer_request_specific(int id)
> {
> @@ -315,7 +314,6 @@ struct omap_dm_timer *omap_dm_timer_request_specific(int id)
>
> return _omap_dm_timer_request(REQUEST_BY_ID, &id);
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_request_specific);
>
> /**
> * omap_dm_timer_request_by_cap - Request a timer by capability
> @@ -330,7 +328,6 @@ struct omap_dm_timer *omap_dm_timer_request_by_cap(u32 cap)
> {
> return _omap_dm_timer_request(REQUEST_BY_CAP, &cap);
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_request_by_cap);
>
> /**
> * omap_dm_timer_request_by_node - Request a timer by device-tree node
> @@ -346,7 +343,6 @@ struct omap_dm_timer *omap_dm_timer_request_by_node(struct device_node *np)
>
> return _omap_dm_timer_request(REQUEST_BY_NODE, np);
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_request_by_node);
>
> int omap_dm_timer_free(struct omap_dm_timer *timer)
> {
> @@ -359,7 +355,6 @@ int omap_dm_timer_free(struct omap_dm_timer *timer)
> timer->reserved = 0;
> return 0;
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_free);
>
> void omap_dm_timer_enable(struct omap_dm_timer *timer)
> {
> @@ -379,13 +374,11 @@ void omap_dm_timer_enable(struct omap_dm_timer *timer)
> }
> }
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_enable);
>
> void omap_dm_timer_disable(struct omap_dm_timer *timer)
> {
> pm_runtime_put_sync(&timer->pdev->dev);
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_disable);
>
> int omap_dm_timer_get_irq(struct omap_dm_timer *timer)
> {
> @@ -393,7 +386,6 @@ int omap_dm_timer_get_irq(struct omap_dm_timer *timer)
> return timer->irq;
> return -EINVAL;
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_get_irq);
>
> #if defined(CONFIG_ARCH_OMAP1)
> #include <mach/hardware.h>
> @@ -429,7 +421,6 @@ __u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)
>
> return inputmask;
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_modify_idlect_mask);
>
> #else
>
> @@ -439,7 +430,6 @@ struct clk *omap_dm_timer_get_fclk(struct omap_dm_timer *timer)
> return timer->fclk;
> return NULL;
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_get_fclk);
>
> __u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)
> {
> @@ -447,7 +437,6 @@ __u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)
>
> return 0;
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_modify_idlect_mask);
>
> #endif
>
> @@ -461,7 +450,6 @@ int omap_dm_timer_trigger(struct omap_dm_timer *timer)
> omap_dm_timer_write_reg(timer, OMAP_TIMER_TRIGGER_REG, 0);
> return 0;
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_trigger);
>
> int omap_dm_timer_start(struct omap_dm_timer *timer)
> {
> @@ -482,7 +470,6 @@ int omap_dm_timer_start(struct omap_dm_timer *timer)
> timer->context.tclr = l;
> return 0;
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_start);
>
> int omap_dm_timer_stop(struct omap_dm_timer *timer)
> {
> @@ -506,7 +493,6 @@ int omap_dm_timer_stop(struct omap_dm_timer *timer)
> omap_dm_timer_disable(timer);
> return 0;
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_stop);
>
> int omap_dm_timer_set_source(struct omap_dm_timer *timer, int source)
> {
> @@ -569,7 +555,6 @@ int omap_dm_timer_set_source(struct omap_dm_timer *timer, int source)
>
> return ret;
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_set_source);
>
> int omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload,
> unsigned int load)
> @@ -595,7 +580,6 @@ int omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload,
> omap_dm_timer_disable(timer);
> return 0;
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_set_load);
>
> /* Optimized set_load which removes costly spin wait in timer_start */
> int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, int autoreload,
> @@ -625,7 +609,6 @@ int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, int autoreload,
> timer->context.tcrr = load;
> return 0;
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_set_load_start);
>
> int omap_dm_timer_set_match(struct omap_dm_timer *timer, int enable,
> unsigned int match)
> @@ -650,7 +633,6 @@ int omap_dm_timer_set_match(struct omap_dm_timer *timer, int enable,
> omap_dm_timer_disable(timer);
> return 0;
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_set_match);
>
> int omap_dm_timer_set_pwm(struct omap_dm_timer *timer, int def_on,
> int toggle, int trigger)
> @@ -676,7 +658,6 @@ int omap_dm_timer_set_pwm(struct omap_dm_timer *timer, int def_on,
> omap_dm_timer_disable(timer);
> return 0;
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_set_pwm);
>
> int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer, int prescaler)
> {
> @@ -699,7 +680,6 @@ int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer, int prescaler)
> omap_dm_timer_disable(timer);
> return 0;
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_set_prescaler);
>
> int omap_dm_timer_set_int_enable(struct omap_dm_timer *timer,
> unsigned int value)
> @@ -716,7 +696,6 @@ int omap_dm_timer_set_int_enable(struct omap_dm_timer *timer,
> omap_dm_timer_disable(timer);
> return 0;
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_set_int_enable);
>
> /**
> * omap_dm_timer_set_int_disable - disable timer interrupts
> @@ -747,7 +726,6 @@ int omap_dm_timer_set_int_disable(struct omap_dm_timer *timer, u32 mask)
> omap_dm_timer_disable(timer);
> return 0;
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_set_int_disable);
>
> unsigned int omap_dm_timer_read_status(struct omap_dm_timer *timer)
> {
> @@ -762,7 +740,6 @@ unsigned int omap_dm_timer_read_status(struct omap_dm_timer *timer)
>
> return l;
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_read_status);
>
> int omap_dm_timer_write_status(struct omap_dm_timer *timer, unsigned int value)
> {
> @@ -773,7 +750,6 @@ int omap_dm_timer_write_status(struct omap_dm_timer *timer, unsigned int value)
>
> return 0;
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_write_status);
>
> unsigned int omap_dm_timer_read_counter(struct omap_dm_timer *timer)
> {
> @@ -784,7 +760,6 @@ unsigned int omap_dm_timer_read_counter(struct omap_dm_timer *timer)
>
> return __omap_dm_timer_read_counter(timer, timer->posted);
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_read_counter);
>
> int omap_dm_timer_write_counter(struct omap_dm_timer *timer, unsigned int value)
> {
> @@ -799,7 +774,6 @@ int omap_dm_timer_write_counter(struct omap_dm_timer *timer, unsigned int value)
> timer->context.tcrr = value;
> return 0;
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timer_write_counter);
>
> int omap_dm_timers_active(void)
> {
> @@ -816,7 +790,6 @@ int omap_dm_timers_active(void)
> }
> return 0;
> }
> -EXPORT_SYMBOL_GPL(omap_dm_timers_active);
>
> static const struct of_device_id omap_timer_match[];
>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2017-12-12 07:32:56

by Keerthy

[permalink] [raw]
Subject: Re: [PATCH v5 1/8] clocksource: dmtimer: Remove all the exports



On Tuesday 12 December 2017 12:46 PM, Ladislav Michl wrote:
> Keerthy,
>
> On Tue, Dec 12, 2017 at 11:42:10AM +0530, Keerthy wrote:
>> Remove all the unwanted exports from the driver
>
> I'm adding event capture capability to the pwm-omap driver and so far used
> v4.15-rc3 as codebase.
>
> Intended use is an IR receiver; for that I need to measure pulses width and
> spaces between pulses. So DM timer was setup to generate interupt after
> both TCAR1 and TCAR2 are filled, values are passed to IR decoder and
> TCAR_IT_FLAG is cleared.
>
> Of course, this is just proof of concept and needs to be polished and
> generalized, but to make it at least work I need functions you just
> unexported (plus some new).
>
> Question is whenever we need this level of indirection (omap_dm_timer_ops)
> or plain exports are enough.

The general guidance is not to do plain exports and go via
omap_dm_timer_ops.

>
> Thank you,
> ladis
>
>> Signed-off-by: Keerthy <[email protected]>
>> Reviewed-by: Sebastian Reichel <[email protected]>
>> ---
>> Changes in v3:
>>
>> * Added Sebastian's Reviewed-by.
>>
>> Changes in v2:
>>
>> * No code changes in this v2 version. Only enhanced patch
>> statistics for renames.
>>
>> arch/arm/plat-omap/dmtimer.c | 27 ---------------------------
>> 1 file changed, 27 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
>> index d443e48..72565fc 100644
>> --- a/arch/arm/plat-omap/dmtimer.c
>> +++ b/arch/arm/plat-omap/dmtimer.c
>> @@ -302,7 +302,6 @@ struct omap_dm_timer *omap_dm_timer_request(void)
>> {
>> return _omap_dm_timer_request(REQUEST_ANY, NULL);
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_request);
>>
>> struct omap_dm_timer *omap_dm_timer_request_specific(int id)
>> {
>> @@ -315,7 +314,6 @@ struct omap_dm_timer *omap_dm_timer_request_specific(int id)
>>
>> return _omap_dm_timer_request(REQUEST_BY_ID, &id);
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_request_specific);
>>
>> /**
>> * omap_dm_timer_request_by_cap - Request a timer by capability
>> @@ -330,7 +328,6 @@ struct omap_dm_timer *omap_dm_timer_request_by_cap(u32 cap)
>> {
>> return _omap_dm_timer_request(REQUEST_BY_CAP, &cap);
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_request_by_cap);
>>
>> /**
>> * omap_dm_timer_request_by_node - Request a timer by device-tree node
>> @@ -346,7 +343,6 @@ struct omap_dm_timer *omap_dm_timer_request_by_node(struct device_node *np)
>>
>> return _omap_dm_timer_request(REQUEST_BY_NODE, np);
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_request_by_node);
>>
>> int omap_dm_timer_free(struct omap_dm_timer *timer)
>> {
>> @@ -359,7 +355,6 @@ int omap_dm_timer_free(struct omap_dm_timer *timer)
>> timer->reserved = 0;
>> return 0;
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_free);
>>
>> void omap_dm_timer_enable(struct omap_dm_timer *timer)
>> {
>> @@ -379,13 +374,11 @@ void omap_dm_timer_enable(struct omap_dm_timer *timer)
>> }
>> }
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_enable);
>>
>> void omap_dm_timer_disable(struct omap_dm_timer *timer)
>> {
>> pm_runtime_put_sync(&timer->pdev->dev);
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_disable);
>>
>> int omap_dm_timer_get_irq(struct omap_dm_timer *timer)
>> {
>> @@ -393,7 +386,6 @@ int omap_dm_timer_get_irq(struct omap_dm_timer *timer)
>> return timer->irq;
>> return -EINVAL;
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_get_irq);
>>
>> #if defined(CONFIG_ARCH_OMAP1)
>> #include <mach/hardware.h>
>> @@ -429,7 +421,6 @@ __u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)
>>
>> return inputmask;
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_modify_idlect_mask);
>>
>> #else
>>
>> @@ -439,7 +430,6 @@ struct clk *omap_dm_timer_get_fclk(struct omap_dm_timer *timer)
>> return timer->fclk;
>> return NULL;
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_get_fclk);
>>
>> __u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)
>> {
>> @@ -447,7 +437,6 @@ __u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)
>>
>> return 0;
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_modify_idlect_mask);
>>
>> #endif
>>
>> @@ -461,7 +450,6 @@ int omap_dm_timer_trigger(struct omap_dm_timer *timer)
>> omap_dm_timer_write_reg(timer, OMAP_TIMER_TRIGGER_REG, 0);
>> return 0;
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_trigger);
>>
>> int omap_dm_timer_start(struct omap_dm_timer *timer)
>> {
>> @@ -482,7 +470,6 @@ int omap_dm_timer_start(struct omap_dm_timer *timer)
>> timer->context.tclr = l;
>> return 0;
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_start);
>>
>> int omap_dm_timer_stop(struct omap_dm_timer *timer)
>> {
>> @@ -506,7 +493,6 @@ int omap_dm_timer_stop(struct omap_dm_timer *timer)
>> omap_dm_timer_disable(timer);
>> return 0;
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_stop);
>>
>> int omap_dm_timer_set_source(struct omap_dm_timer *timer, int source)
>> {
>> @@ -569,7 +555,6 @@ int omap_dm_timer_set_source(struct omap_dm_timer *timer, int source)
>>
>> return ret;
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_set_source);
>>
>> int omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload,
>> unsigned int load)
>> @@ -595,7 +580,6 @@ int omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload,
>> omap_dm_timer_disable(timer);
>> return 0;
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_set_load);
>>
>> /* Optimized set_load which removes costly spin wait in timer_start */
>> int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, int autoreload,
>> @@ -625,7 +609,6 @@ int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, int autoreload,
>> timer->context.tcrr = load;
>> return 0;
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_set_load_start);
>>
>> int omap_dm_timer_set_match(struct omap_dm_timer *timer, int enable,
>> unsigned int match)
>> @@ -650,7 +633,6 @@ int omap_dm_timer_set_match(struct omap_dm_timer *timer, int enable,
>> omap_dm_timer_disable(timer);
>> return 0;
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_set_match);
>>
>> int omap_dm_timer_set_pwm(struct omap_dm_timer *timer, int def_on,
>> int toggle, int trigger)
>> @@ -676,7 +658,6 @@ int omap_dm_timer_set_pwm(struct omap_dm_timer *timer, int def_on,
>> omap_dm_timer_disable(timer);
>> return 0;
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_set_pwm);
>>
>> int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer, int prescaler)
>> {
>> @@ -699,7 +680,6 @@ int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer, int prescaler)
>> omap_dm_timer_disable(timer);
>> return 0;
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_set_prescaler);
>>
>> int omap_dm_timer_set_int_enable(struct omap_dm_timer *timer,
>> unsigned int value)
>> @@ -716,7 +696,6 @@ int omap_dm_timer_set_int_enable(struct omap_dm_timer *timer,
>> omap_dm_timer_disable(timer);
>> return 0;
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_set_int_enable);
>>
>> /**
>> * omap_dm_timer_set_int_disable - disable timer interrupts
>> @@ -747,7 +726,6 @@ int omap_dm_timer_set_int_disable(struct omap_dm_timer *timer, u32 mask)
>> omap_dm_timer_disable(timer);
>> return 0;
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_set_int_disable);
>>
>> unsigned int omap_dm_timer_read_status(struct omap_dm_timer *timer)
>> {
>> @@ -762,7 +740,6 @@ unsigned int omap_dm_timer_read_status(struct omap_dm_timer *timer)
>>
>> return l;
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_read_status);
>>
>> int omap_dm_timer_write_status(struct omap_dm_timer *timer, unsigned int value)
>> {
>> @@ -773,7 +750,6 @@ int omap_dm_timer_write_status(struct omap_dm_timer *timer, unsigned int value)
>>
>> return 0;
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_write_status);
>>
>> unsigned int omap_dm_timer_read_counter(struct omap_dm_timer *timer)
>> {
>> @@ -784,7 +760,6 @@ unsigned int omap_dm_timer_read_counter(struct omap_dm_timer *timer)
>>
>> return __omap_dm_timer_read_counter(timer, timer->posted);
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_read_counter);
>>
>> int omap_dm_timer_write_counter(struct omap_dm_timer *timer, unsigned int value)
>> {
>> @@ -799,7 +774,6 @@ int omap_dm_timer_write_counter(struct omap_dm_timer *timer, unsigned int value)
>> timer->context.tcrr = value;
>> return 0;
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timer_write_counter);
>>
>> int omap_dm_timers_active(void)
>> {
>> @@ -816,7 +790,6 @@ int omap_dm_timers_active(void)
>> }
>> return 0;
>> }
>> -EXPORT_SYMBOL_GPL(omap_dm_timers_active);
>>
>> static const struct of_device_id omap_timer_match[];
>>
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html

2017-12-12 08:01:41

by Ladislav Michl

[permalink] [raw]
Subject: Re: [PATCH v5 1/8] clocksource: dmtimer: Remove all the exports

On Tue, Dec 12, 2017 at 01:01:51PM +0530, Keerthy wrote:
>
>
> On Tuesday 12 December 2017 12:46 PM, Ladislav Michl wrote:
> > Keerthy,
> >
> > On Tue, Dec 12, 2017 at 11:42:10AM +0530, Keerthy wrote:
> >> Remove all the unwanted exports from the driver
> >
> > I'm adding event capture capability to the pwm-omap driver and so far used
> > v4.15-rc3 as codebase.
> >
> > Intended use is an IR receiver; for that I need to measure pulses width and
> > spaces between pulses. So DM timer was setup to generate interupt after
> > both TCAR1 and TCAR2 are filled, values are passed to IR decoder and
> > TCAR_IT_FLAG is cleared.
> >
> > Of course, this is just proof of concept and needs to be polished and
> > generalized, but to make it at least work I need functions you just
> > unexported (plus some new).
> >
> > Question is whenever we need this level of indirection (omap_dm_timer_ops)
> > or plain exports are enough.
>
> The general guidance is not to do plain exports and go via
> omap_dm_timer_ops.

...in contrary what other clocksource drivers are doing.

Now I'm assuming it is okay to extend omap_dm_timer_ops. That would mean
check for ops members to be assigned should be also extended or we should
delete it altogether and assume all members are populated?

Thanks,
ladis

2017-12-12 08:09:12

by Keerthy

[permalink] [raw]
Subject: Re: [PATCH v5 1/8] clocksource: dmtimer: Remove all the exports



On Tuesday 12 December 2017 01:31 PM, Ladislav Michl wrote:
> On Tue, Dec 12, 2017 at 01:01:51PM +0530, Keerthy wrote:
>>
>>
>> On Tuesday 12 December 2017 12:46 PM, Ladislav Michl wrote:
>>> Keerthy,
>>>
>>> On Tue, Dec 12, 2017 at 11:42:10AM +0530, Keerthy wrote:
>>>> Remove all the unwanted exports from the driver
>>>
>>> I'm adding event capture capability to the pwm-omap driver and so far used
>>> v4.15-rc3 as codebase.
>>>
>>> Intended use is an IR receiver; for that I need to measure pulses width and
>>> spaces between pulses. So DM timer was setup to generate interupt after
>>> both TCAR1 and TCAR2 are filled, values are passed to IR decoder and
>>> TCAR_IT_FLAG is cleared.
>>>
>>> Of course, this is just proof of concept and needs to be polished and
>>> generalized, but to make it at least work I need functions you just
>>> unexported (plus some new).
>>>
>>> Question is whenever we need this level of indirection (omap_dm_timer_ops)
>>> or plain exports are enough.
>>
>> The general guidance is not to do plain exports and go via
>> omap_dm_timer_ops.
>
> ...in contrary what other clocksource drivers are doing.
>
> Now I'm assuming it is okay to extend omap_dm_timer_ops. That would mean
> check for ops members to be assigned should be also extended or we should
> delete it altogether and assume all members are populated?

It should be fine to extend omap_dm_timer_ops. What are the ops missing
for your new implementation?

Tony,

Your thoughts on the above?

Regards,
Keerthy
>
> Thanks,
> ladis
>

2017-12-12 08:19:32

by Ladislav Michl

[permalink] [raw]
Subject: Re: [PATCH v5 1/8] clocksource: dmtimer: Remove all the exports

On Tue, Dec 12, 2017 at 01:38:04PM +0530, Keerthy wrote:
> On Tuesday 12 December 2017 01:31 PM, Ladislav Michl wrote:
> > On Tue, Dec 12, 2017 at 01:01:51PM +0530, Keerthy wrote:
> >>
> >>
> >> On Tuesday 12 December 2017 12:46 PM, Ladislav Michl wrote:
> >>> Keerthy,
> >>>
> >>> On Tue, Dec 12, 2017 at 11:42:10AM +0530, Keerthy wrote:
> >>>> Remove all the unwanted exports from the driver
> >>>
> >>> I'm adding event capture capability to the pwm-omap driver and so far used
> >>> v4.15-rc3 as codebase.
> >>>
> >>> Intended use is an IR receiver; for that I need to measure pulses width and
> >>> spaces between pulses. So DM timer was setup to generate interupt after
> >>> both TCAR1 and TCAR2 are filled, values are passed to IR decoder and
> >>> TCAR_IT_FLAG is cleared.
> >>>
> >>> Of course, this is just proof of concept and needs to be polished and
> >>> generalized, but to make it at least work I need functions you just
> >>> unexported (plus some new).
> >>>
> >>> Question is whenever we need this level of indirection (omap_dm_timer_ops)
> >>> or plain exports are enough.
> >>
> >> The general guidance is not to do plain exports and go via
> >> omap_dm_timer_ops.
> >
> > ...in contrary what other clocksource drivers are doing.
> >
> > Now I'm assuming it is okay to extend omap_dm_timer_ops. That would mean
> > check for ops members to be assigned should be also extended or we should
> > delete it altogether and assume all members are populated?
>
> It should be fine to extend omap_dm_timer_ops. What are the ops missing
> for your new implementation?

Read capture registers, configure capture and ack interrupt. Perhaps set_pwm
could be extended to configure capture as well.

I'll update my code on top of your changes and we'll see how it would work.

> Tony,
>
> Your thoughts on the above?
>
> R

2017-12-12 08:25:14

by Keerthy

[permalink] [raw]
Subject: Re: [PATCH v5 1/8] clocksource: dmtimer: Remove all the exports



On Tuesday 12 December 2017 01:49 PM, Ladislav Michl wrote:
> On Tue, Dec 12, 2017 at 01:38:04PM +0530, Keerthy wrote:
>> On Tuesday 12 December 2017 01:31 PM, Ladislav Michl wrote:
>>> On Tue, Dec 12, 2017 at 01:01:51PM +0530, Keerthy wrote:
>>>>
>>>>
>>>> On Tuesday 12 December 2017 12:46 PM, Ladislav Michl wrote:
>>>>> Keerthy,
>>>>>
>>>>> On Tue, Dec 12, 2017 at 11:42:10AM +0530, Keerthy wrote:
>>>>>> Remove all the unwanted exports from the driver
>>>>>
>>>>> I'm adding event capture capability to the pwm-omap driver and so far used
>>>>> v4.15-rc3 as codebase.
>>>>>
>>>>> Intended use is an IR receiver; for that I need to measure pulses width and
>>>>> spaces between pulses. So DM timer was setup to generate interupt after
>>>>> both TCAR1 and TCAR2 are filled, values are passed to IR decoder and
>>>>> TCAR_IT_FLAG is cleared.
>>>>>
>>>>> Of course, this is just proof of concept and needs to be polished and
>>>>> generalized, but to make it at least work I need functions you just
>>>>> unexported (plus some new).
>>>>>
>>>>> Question is whenever we need this level of indirection (omap_dm_timer_ops)
>>>>> or plain exports are enough.
>>>>
>>>> The general guidance is not to do plain exports and go via
>>>> omap_dm_timer_ops.
>>>
>>> ...in contrary what other clocksource drivers are doing.
>>>
>>> Now I'm assuming it is okay to extend omap_dm_timer_ops. That would mean
>>> check for ops members to be assigned should be also extended or we should
>>> delete it altogether and assume all members are populated?
>>
>> It should be fine to extend omap_dm_timer_ops. What are the ops missing
>> for your new implementation?
>
> Read capture registers, configure capture and ack interrupt. Perhaps set_pwm
> could be extended to configure capture as well.
>
> I'll update my code on top of your changes and we'll see how it would work.

Okay Thanks!

>
>> Tony,
>>
>> Your thoughts on the above?
>>
>> R

2017-12-12 17:01:05

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH v5 1/8] clocksource: dmtimer: Remove all the exports

* Keerthy <[email protected]> [171212 08:25]:
>
>
> On Tuesday 12 December 2017 01:49 PM, Ladislav Michl wrote:
> > On Tue, Dec 12, 2017 at 01:38:04PM +0530, Keerthy wrote:
> >> On Tuesday 12 December 2017 01:31 PM, Ladislav Michl wrote:
> >>> On Tue, Dec 12, 2017 at 01:01:51PM +0530, Keerthy wrote:
> >>>>
> >>>>
> >>>> On Tuesday 12 December 2017 12:46 PM, Ladislav Michl wrote:
> >>>>> Keerthy,
> >>>>>
> >>>>> On Tue, Dec 12, 2017 at 11:42:10AM +0530, Keerthy wrote:
> >>>>>> Remove all the unwanted exports from the driver
> >>>>>
> >>>>> I'm adding event capture capability to the pwm-omap driver and so far used
> >>>>> v4.15-rc3 as codebase.
> >>>>>
> >>>>> Intended use is an IR receiver; for that I need to measure pulses width and
> >>>>> spaces between pulses. So DM timer was setup to generate interupt after
> >>>>> both TCAR1 and TCAR2 are filled, values are passed to IR decoder and
> >>>>> TCAR_IT_FLAG is cleared.
> >>>>>
> >>>>> Of course, this is just proof of concept and needs to be polished and
> >>>>> generalized, but to make it at least work I need functions you just
> >>>>> unexported (plus some new).
> >>>>>
> >>>>> Question is whenever we need this level of indirection (omap_dm_timer_ops)
> >>>>> or plain exports are enough.
> >>>>
> >>>> The general guidance is not to do plain exports and go via
> >>>> omap_dm_timer_ops.
> >>>
> >>> ...in contrary what other clocksource drivers are doing.

Hmm what do you mean? We don't want to export tons of custom functions from
the timers in and then be in trouble when at some point we have a Linux
generic hw timer framework. We already had to deal with these custom
exports earlier with conversion to multiarch and then again with
device tree.

For now, it's best to pass the timer information to the pwm driver in
platform data. In the long run that will be much easier to deal with than
fixing random drivers tinkering with the timer registers directly.

> >>> Now I'm assuming it is okay to extend omap_dm_timer_ops. That would mean
> >>> check for ops members to be assigned should be also extended or we should
> >>> delete it altogether and assume all members are populated?
> >>
> >> It should be fine to extend omap_dm_timer_ops. What are the ops missing
> >> for your new implementation?
> >
> > Read capture registers, configure capture and ack interrupt. Perhaps set_pwm
> > could be extended to configure capture as well.
> >
> > I'll update my code on top of your changes and we'll see how it would work.

Ideally the pwm driver would just do a request_irq from the dmtimer code
where dmtimer code would implement an interrupt controller. That would
be already most fo the Linux generic hardware timer framework right there :)

Regards,

Tony

2017-12-12 18:03:43

by Ladislav Michl

[permalink] [raw]
Subject: Re: [PATCH v5 1/8] clocksource: dmtimer: Remove all the exports

On Tue, Dec 12, 2017 at 09:00:54AM -0800, Tony Lindgren wrote:
> Hmm what do you mean? We don't want to export tons of custom functions from
> the timers in and then be in trouble when at some point we have a Linux
> generic hw timer framework. We already had to deal with these custom
> exports earlier with conversion to multiarch and then again with
> device tree.
>
> For now, it's best to pass the timer information to the pwm driver in
> platform data. In the long run that will be much easier to deal with than
> fixing random drivers tinkering with the timer registers directly.

All that register access would happen only in drivers/clocksource/timer-dm.c?
So platform data will hold all function pointers needed for event capture
and the pwm driver will do only interface to pwm framework.

> Ideally the pwm driver would just do a request_irq from the dmtimer code
> where dmtimer code would implement an interrupt controller. That would
> be already most fo the Linux generic hardware timer framework right there :)

I do not follow. Each general-purpose timer module has its own interrupt line,
so claiming that irq directly using request_irq seems enough. Could you
explain interrupt controller idea a bit more?

Thank you,
ladis

2017-12-12 18:21:58

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH v5 1/8] clocksource: dmtimer: Remove all the exports

* Ladislav Michl <[email protected]> [171212 18:06]:
> On Tue, Dec 12, 2017 at 09:00:54AM -0800, Tony Lindgren wrote:
> > Hmm what do you mean? We don't want to export tons of custom functions from
> > the timers in and then be in trouble when at some point we have a Linux
> > generic hw timer framework. We already had to deal with these custom
> > exports earlier with conversion to multiarch and then again with
> > device tree.
> >
> > For now, it's best to pass the timer information to the pwm driver in
> > platform data. In the long run that will be much easier to deal with than
> > fixing random drivers tinkering with the timer registers directly.
>
> All that register access would happen only in drivers/clocksource/timer-dm.c?
> So platform data will hold all function pointers needed for event capture
> and the pwm driver will do only interface to pwm framework.

Yes please.

> > Ideally the pwm driver would just do a request_irq from the dmtimer code
> > where dmtimer code would implement an interrupt controller. That would
> > be already most fo the Linux generic hardware timer framework right there :)
>
> I do not follow. Each general-purpose timer module has its own interrupt line,
> so claiming that irq directly using request_irq seems enough. Could you
> explain interrupt controller idea a bit more?

Well let's assume we have drivers/clocksource/timer-dm.c implement
an irq controller. Then the pwm driver would just do:

pwm9: dmtimer-pwm {
compatible = "ti,omap-dmtimer-pwm";
#pwm-cells = <3>;
ti,timers = <&timer9>;
ti,clock-source = <0x00>; /* timer_sys_ck */
interrupts-extended = <&timer9 IRQ_TYPE_SOMETHING>;
};

Then you can do whatever you need to in the pwm driver with
enable_irq/disable_irq + a handler?

If reading the line status is needed.. Then maybe the GPIO framework
needs to have hardware timer support instead?

Anyways, just thinking out loud how we could have a Linux generic
hardware timer framework that drivers like pwm could then use.

Regards,

Tony

2017-12-13 09:15:36

by Ladislav Michl

[permalink] [raw]
Subject: Re: [PATCH v5 1/8] clocksource: dmtimer: Remove all the exports

On Tue, Dec 12, 2017 at 10:21:50AM -0800, Tony Lindgren wrote:
> * Ladislav Michl <[email protected]> [171212 18:06]:
> > I do not follow. Each general-purpose timer module has its own interrupt line,
> > so claiming that irq directly using request_irq seems enough. Could you
> > explain interrupt controller idea a bit more?
>
> Well let's assume we have drivers/clocksource/timer-dm.c implement
> an irq controller. Then the pwm driver would just do:
>
> pwm9: dmtimer-pwm {
> compatible = "ti,omap-dmtimer-pwm";
> #pwm-cells = <3>;
> ti,timers = <&timer9>;
> ti,clock-source = <0x00>; /* timer_sys_ck */
> interrupts-extended = <&timer9 IRQ_TYPE_SOMETHING>;
> };
>
> Then you can do whatever you need to in the pwm driver with
> enable_irq/disable_irq + a handler?

That seems to work. Now should we map 1:1 to timer interrupt or
have separate interrupt for match, overflow and capture?
Former would need some more dm_timer_ops to determine interrupt
source, while later would work "automagically" - but I haven't
tested it yet.

> If reading the line status is needed.. Then maybe the GPIO framework
> needs to have hardware timer support instead?

It does not seem OMAP can read event pin value in event capture mode.

> Anyways, just thinking out loud how we could have a Linux generic
> hardware timer framework that drivers like pwm could then use.

I need a bit longer chain:
dmtimer -> pwm -> rc (which calls ir_raw_event_store from interrupt)
Is extending pwm core with interrpt callback the right thing there?
Something like:
(*pulse_captured)(ktime_t width, ktime_t last_edge);

Thank you,
ladis

2017-12-13 16:51:20

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH v5 1/8] clocksource: dmtimer: Remove all the exports

* Ladislav Michl <[email protected]> [171213 09:17]:
> On Tue, Dec 12, 2017 at 10:21:50AM -0800, Tony Lindgren wrote:
> > * Ladislav Michl <[email protected]> [171212 18:06]:
> > > I do not follow. Each general-purpose timer module has its own interrupt line,
> > > so claiming that irq directly using request_irq seems enough. Could you
> > > explain interrupt controller idea a bit more?
> >
> > Well let's assume we have drivers/clocksource/timer-dm.c implement
> > an irq controller. Then the pwm driver would just do:
> >
> > pwm9: dmtimer-pwm {
> > compatible = "ti,omap-dmtimer-pwm";
> > #pwm-cells = <3>;
> > ti,timers = <&timer9>;
> > ti,clock-source = <0x00>; /* timer_sys_ck */
> > interrupts-extended = <&timer9 IRQ_TYPE_SOMETHING>;
> > };
> >
> > Then you can do whatever you need to in the pwm driver with
> > enable_irq/disable_irq + a handler?
>
> That seems to work. Now should we map 1:1 to timer interrupt or
> have separate interrupt for match, overflow and capture?
> Former would need some more dm_timer_ops to determine interrupt
> source, while later would work "automagically" - but I haven't
> tested it yet.

Hmm the second option sounds appealing to me as then the device
tree binding really follows the hardware:

bit 2 capture interrupt
bit 1 overflow interrupt
bit 0 match interrupt

and in the dts we can describe these with:

interrupts-extended = <&timer 9 2 IRQ_TYPE_EDGE_SOMETHING>;
interrupts-extended = <&timer 9 1 IRQ_TYPE_EDGE_SOMETHING>;
interrupts-extended = <&timer 9 0 IRQ_TYPE_EDGE_SOMETHING>;

I think these are all edge based on "Capture Mode Functionality"
chapter in the TRM?

> > If reading the line status is needed.. Then maybe the GPIO framework
> > needs to have hardware timer support instead?
>
> It does not seem OMAP can read event pin value in event capture mode.

Sounds like a bug somehwere, probably in software?

> > Anyways, just thinking out loud how we could have a Linux generic
> > hardware timer framework that drivers like pwm could then use.
>
> I need a bit longer chain:
> dmtimer -> pwm -> rc (which calls ir_raw_event_store from interrupt)
> Is extending pwm core with interrpt callback the right thing there?
> Something like:
> (*pulse_captured)(ktime_t width, ktime_t last_edge);

OK seems like having drivers/clocksource/timer-dm.c a chained
interrupt handler should do it :) Anyways, that's a different
set of patches on top of these.

Regards,

Tony

2017-12-18 09:31:59

by Ladislav Michl

[permalink] [raw]
Subject: Re: [PATCH v5 7/8] pwm: pwm-omap-dmtimer: Adapt driver to utilize dmtimer pdata ops

Keerthy,

On Tue, Dec 12, 2017 at 11:42:16AM +0530, Keerthy wrote:
> Adapt driver to utilize dmtimer pdata ops instead of pdata-quirks.
>
> Signed-off-by: Keerthy <[email protected]>
> ---
>
> Changes in v4:
>
> * Switched to dev_get_platdata.

Where do you expect dev.platform_data to be set? PWM driver is failing
with:
omap-dmtimer-pwm dmtimer-pwm: dmtimer pdata structure NULL
omap-dmtimer-pwm: probe of dmtimer-pwm failed with error -22

Which I fixed with patch bellow, to be able to test your patchset.

Also I'm running a bit out of time, so I'll send few clean up
patches and event capture code to get some feedback early.

Regards,
ladis

diff --git a/drivers/clocksource/timer-dm.c b/drivers/clocksource/timer-dm.c
index 39be39e6a8dd..d3d8a49cae0d 100644
--- a/drivers/clocksource/timer-dm.c
+++ b/drivers/clocksource/timer-dm.c
@@ -773,6 +773,7 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
dev_err(dev, "%s: no platform data.\n", __func__);
return -ENODEV;
}
+ dev->platform_data = pdata;

irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
if (unlikely(!irq)) {

>
> Changes in v3:
>
> * Used of_find_platdata_by_node function to fetch platform
> data for timer node.
>
> drivers/pwm/pwm-omap-dmtimer.c | 39 ++++++++++++++++++++++-----------------
> 1 file changed, 22 insertions(+), 17 deletions(-)

2017-12-18 11:16:15

by Ladislav Michl

[permalink] [raw]
Subject: Re: [PATCH v5 0/8] omap: dmtimer: Move driver out of plat-omap

Keerthy,

On Tue, Dec 12, 2017 at 11:42:09AM +0530, Keerthy wrote:
> The series moves dmtimer out of plat-omap to drivers/clocksource.
> The series also does a bunch of changes to pwm-omap-dmtimer code
> to adapt to the driver migration and clean up plat specific
> pdata-quirks and use the dmtimer platform data.

thanks for nice work. I'll send two more patches as a reply to this
one. I'd be glad if you could make them part of your serie. One of
them would be nice to have for pwm driver prescaler fix which will
be send independently.
Also, if that helps you can have my
Tested-by: Ladislav Michl <[email protected]>
on IGEPv2 (OMAP3430 based)

> Boot tested on DRA7-EVM and AM437X-GP-EVM.

I guess PWM driver was not tested, right? See comment to PATCH 7/8.

> Compile tested omap1_defconfig.

Thank you,
ladis

2017-12-18 11:31:01

by Ladislav Michl

[permalink] [raw]
Subject: [PATCH 1/2] clocksource: timer-dm: Make unexported functions static

As dmtimer no longer exports functions, there is no point
to have them non-static. Also delete those not used anywhere.

Signed-off-by: Ladislav Michl <[email protected]>
---
drivers/clocksource/timer-dm.c | 125 ++++++++---------------------------------
include/clocksource/dmtimer.h | 26 ---------
2 files changed, 23 insertions(+), 128 deletions(-)

diff --git a/drivers/clocksource/timer-dm.c b/drivers/clocksource/timer-dm.c
index 392978ccd8a6..ec3a28c90c70 100644
--- a/drivers/clocksource/timer-dm.c
+++ b/drivers/clocksource/timer-dm.c
@@ -204,16 +204,6 @@ static inline u32 omap_dm_timer_reserved_systimer(int id)
return (omap_reserved_systimers & (1 << (id - 1))) ? 1 : 0;
}

-int omap_dm_timer_reserve_systimer(int id)
-{
- if (omap_dm_timer_reserved_systimer(id))
- return -ENODEV;
-
- omap_reserved_systimers |= (1 << (id - 1));
-
- return 0;
-}
-
static struct omap_dm_timer *_omap_dm_timer_request(int req_type, void *data)
{
struct omap_dm_timer *timer = NULL, *t;
@@ -298,16 +288,16 @@ static struct omap_dm_timer *_omap_dm_timer_request(int req_type, void *data)
return timer;
}

-struct omap_dm_timer *omap_dm_timer_request(void)
+static struct omap_dm_timer *omap_dm_timer_request(void)
{
return _omap_dm_timer_request(REQUEST_ANY, NULL);
}

-struct omap_dm_timer *omap_dm_timer_request_specific(int id)
+static struct omap_dm_timer *omap_dm_timer_request_specific(int id)
{
/* Requesting timer by ID is not supported when device tree is used */
if (of_have_populated_dt()) {
- pr_warn("%s: Please use omap_dm_timer_request_by_cap/node()\n",
+ pr_warn("%s: Please use omap_dm_timer_request_by_node()\n",
__func__);
return NULL;
}
@@ -315,20 +305,6 @@ struct omap_dm_timer *omap_dm_timer_request_specific(int id)
return _omap_dm_timer_request(REQUEST_BY_ID, &id);
}

-/**
- * omap_dm_timer_request_by_cap - Request a timer by capability
- * @cap: Bit mask of capabilities to match
- *
- * Find a timer based upon capabilities bit mask. Callers of this function
- * should use the definitions found in the plat/dmtimer.h file under the
- * comment "timer capabilities used in hwmod database". Returns pointer to
- * timer handle on success and a NULL pointer on failure.
- */
-struct omap_dm_timer *omap_dm_timer_request_by_cap(u32 cap)
-{
- return _omap_dm_timer_request(REQUEST_BY_CAP, &cap);
-}
-
/**
* omap_dm_timer_request_by_node - Request a timer by device-tree node
* @np: Pointer to device-tree timer node
@@ -336,7 +312,7 @@ struct omap_dm_timer *omap_dm_timer_request_by_cap(u32 cap)
* Request a timer based upon a device node pointer. Returns pointer to
* timer handle on success and a NULL pointer on failure.
*/
-struct omap_dm_timer *omap_dm_timer_request_by_node(struct device_node *np)
+static struct omap_dm_timer *omap_dm_timer_request_by_node(struct device_node *np)
{
if (!np)
return NULL;
@@ -344,7 +320,7 @@ struct omap_dm_timer *omap_dm_timer_request_by_node(struct device_node *np)
return _omap_dm_timer_request(REQUEST_BY_NODE, np);
}

-int omap_dm_timer_free(struct omap_dm_timer *timer)
+static int omap_dm_timer_free(struct omap_dm_timer *timer)
{
if (unlikely(!timer))
return -EINVAL;
@@ -424,7 +400,7 @@ __u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)

#else

-struct clk *omap_dm_timer_get_fclk(struct omap_dm_timer *timer)
+static struct clk *omap_dm_timer_get_fclk(struct omap_dm_timer *timer)
{
if (timer && !IS_ERR(timer->fclk))
return timer->fclk;
@@ -440,18 +416,7 @@ __u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)

#endif

-int omap_dm_timer_trigger(struct omap_dm_timer *timer)
-{
- if (unlikely(!timer || pm_runtime_suspended(&timer->pdev->dev))) {
- pr_err("%s: timer not available or enabled.\n", __func__);
- return -EINVAL;
- }
-
- omap_dm_timer_write_reg(timer, OMAP_TIMER_TRIGGER_REG, 0);
- return 0;
-}
-
-int omap_dm_timer_start(struct omap_dm_timer *timer)
+static int omap_dm_timer_start(struct omap_dm_timer *timer)
{
u32 l;

@@ -471,7 +436,7 @@ int omap_dm_timer_start(struct omap_dm_timer *timer)
return 0;
}

-int omap_dm_timer_stop(struct omap_dm_timer *timer)
+static int omap_dm_timer_stop(struct omap_dm_timer *timer)
{
unsigned long rate = 0;

@@ -556,8 +521,8 @@ int omap_dm_timer_set_source(struct omap_dm_timer *timer, int source)
return ret;
}

-int omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload,
- unsigned int load)
+static int omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload,
+ unsigned int load)
{
u32 l;

@@ -581,37 +546,8 @@ int omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload,
return 0;
}

-/* Optimized set_load which removes costly spin wait in timer_start */
-int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, int autoreload,
- unsigned int load)
-{
- u32 l;
-
- if (unlikely(!timer))
- return -EINVAL;
-
- omap_dm_timer_enable(timer);
-
- l = omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
- if (autoreload) {
- l |= OMAP_TIMER_CTRL_AR;
- omap_dm_timer_write_reg(timer, OMAP_TIMER_LOAD_REG, load);
- } else {
- l &= ~OMAP_TIMER_CTRL_AR;
- }
- l |= OMAP_TIMER_CTRL_ST;
-
- __omap_dm_timer_load_start(timer, l, load, timer->posted);
-
- /* Save the context */
- timer->context.tclr = l;
- timer->context.tldr = load;
- timer->context.tcrr = load;
- return 0;
-}
-
-int omap_dm_timer_set_match(struct omap_dm_timer *timer, int enable,
- unsigned int match)
+static int omap_dm_timer_set_match(struct omap_dm_timer *timer, int enable,
+ unsigned int match)
{
u32 l;

@@ -634,8 +570,8 @@ int omap_dm_timer_set_match(struct omap_dm_timer *timer, int enable,
return 0;
}

-int omap_dm_timer_set_pwm(struct omap_dm_timer *timer, int def_on,
- int toggle, int trigger)
+static int omap_dm_timer_set_pwm(struct omap_dm_timer *timer, int def_on,
+ int toggle, int trigger)
{
u32 l;

@@ -659,7 +595,8 @@ int omap_dm_timer_set_pwm(struct omap_dm_timer *timer, int def_on,
return 0;
}

-int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer, int prescaler)
+static int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer,
+ int prescaler)
{
u32 l;

@@ -681,8 +618,8 @@ int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer, int prescaler)
return 0;
}

-int omap_dm_timer_set_int_enable(struct omap_dm_timer *timer,
- unsigned int value)
+static int omap_dm_timer_set_int_enable(struct omap_dm_timer *timer,
+ unsigned int value)
{
if (unlikely(!timer))
return -EINVAL;
@@ -704,7 +641,7 @@ int omap_dm_timer_set_int_enable(struct omap_dm_timer *timer,
*
* Disables the specified timer interrupts for a timer.
*/
-int omap_dm_timer_set_int_disable(struct omap_dm_timer *timer, u32 mask)
+static int omap_dm_timer_set_int_disable(struct omap_dm_timer *timer, u32 mask)
{
u32 l = mask;

@@ -727,7 +664,7 @@ int omap_dm_timer_set_int_disable(struct omap_dm_timer *timer, u32 mask)
return 0;
}

-unsigned int omap_dm_timer_read_status(struct omap_dm_timer *timer)
+static unsigned int omap_dm_timer_read_status(struct omap_dm_timer *timer)
{
unsigned int l;

@@ -741,7 +678,7 @@ unsigned int omap_dm_timer_read_status(struct omap_dm_timer *timer)
return l;
}

-int omap_dm_timer_write_status(struct omap_dm_timer *timer, unsigned int value)
+static int omap_dm_timer_write_status(struct omap_dm_timer *timer, unsigned int value)
{
if (unlikely(!timer || pm_runtime_suspended(&timer->pdev->dev)))
return -EINVAL;
@@ -751,7 +688,7 @@ int omap_dm_timer_write_status(struct omap_dm_timer *timer, unsigned int value)
return 0;
}

-unsigned int omap_dm_timer_read_counter(struct omap_dm_timer *timer)
+static unsigned int omap_dm_timer_read_counter(struct omap_dm_timer *timer)
{
if (unlikely(!timer || pm_runtime_suspended(&timer->pdev->dev))) {
pr_err("%s: timer not iavailable or enabled.\n", __func__);
@@ -761,7 +698,7 @@ unsigned int omap_dm_timer_read_counter(struct omap_dm_timer *timer)
return __omap_dm_timer_read_counter(timer, timer->posted);
}

-int omap_dm_timer_write_counter(struct omap_dm_timer *timer, unsigned int value)
+static int omap_dm_timer_write_counter(struct omap_dm_timer *timer, unsigned int value)
{
if (unlikely(!timer || pm_runtime_suspended(&timer->pdev->dev))) {
pr_err("%s: timer not available or enabled.\n", __func__);
@@ -775,22 +712,6 @@ int omap_dm_timer_write_counter(struct omap_dm_timer *timer, unsigned int value)
return 0;
}

-int omap_dm_timers_active(void)
-{
- struct omap_dm_timer *timer;
-
- list_for_each_entry(timer, &omap_timer_list, node) {
- if (!timer->reserved)
- continue;
-
- if (omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG) &
- OMAP_TIMER_CTRL_ST) {
- return 1;
- }
- }
- return 0;
-}
-
static const struct of_device_id omap_timer_match[];

/**
diff --git a/include/clocksource/dmtimer.h b/include/clocksource/dmtimer.h
index 862ad62dab9d..63d6ec0747d9 100644
--- a/include/clocksource/dmtimer.h
+++ b/include/clocksource/dmtimer.h
@@ -124,40 +124,14 @@ struct omap_dm_timer {
struct list_head node;
};

-int omap_dm_timer_reserve_systimer(int id);
-struct omap_dm_timer *omap_dm_timer_request(void);
-struct omap_dm_timer *omap_dm_timer_request_specific(int timer_id);
-struct omap_dm_timer *omap_dm_timer_request_by_cap(u32 cap);
-struct omap_dm_timer *omap_dm_timer_request_by_node(struct device_node *np);
-int omap_dm_timer_free(struct omap_dm_timer *timer);
void omap_dm_timer_enable(struct omap_dm_timer *timer);
void omap_dm_timer_disable(struct omap_dm_timer *timer);

int omap_dm_timer_get_irq(struct omap_dm_timer *timer);

u32 omap_dm_timer_modify_idlect_mask(u32 inputmask);
-struct clk *omap_dm_timer_get_fclk(struct omap_dm_timer *timer);
-
-int omap_dm_timer_trigger(struct omap_dm_timer *timer);
-int omap_dm_timer_start(struct omap_dm_timer *timer);
-int omap_dm_timer_stop(struct omap_dm_timer *timer);

int omap_dm_timer_set_source(struct omap_dm_timer *timer, int source);
-int omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload, unsigned int value);
-int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, int autoreload, unsigned int value);
-int omap_dm_timer_set_match(struct omap_dm_timer *timer, int enable, unsigned int match);
-int omap_dm_timer_set_pwm(struct omap_dm_timer *timer, int def_on, int toggle, int trigger);
-int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer, int prescaler);
-
-int omap_dm_timer_set_int_enable(struct omap_dm_timer *timer, unsigned int value);
-int omap_dm_timer_set_int_disable(struct omap_dm_timer *timer, u32 mask);
-
-unsigned int omap_dm_timer_read_status(struct omap_dm_timer *timer);
-int omap_dm_timer_write_status(struct omap_dm_timer *timer, unsigned int value);
-unsigned int omap_dm_timer_read_counter(struct omap_dm_timer *timer);
-int omap_dm_timer_write_counter(struct omap_dm_timer *timer, unsigned int value);
-
-int omap_dm_timers_active(void);

/*
* Do not use the defines below, they are not needed. They should be only
--
2.15.1

2017-12-18 11:31:57

by Ladislav Michl

[permalink] [raw]
Subject: [PATCH 2/2] clocksource: timer-dm: Check prescaler value

Invalid prescaler value is silently ignored. Fix that
by returning -EINVAL in such case. As invalid value
disabled use of the prescaler, use -1 explicitely for
that purpose.

Signed-off-by: Ladislav Michl <[email protected]>
---
drivers/clocksource/timer-dm.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/clocksource/timer-dm.c b/drivers/clocksource/timer-dm.c
index ec3a28c90c70..95cd98be8541 100644
--- a/drivers/clocksource/timer-dm.c
+++ b/drivers/clocksource/timer-dm.c
@@ -609,6 +609,9 @@ static int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer,
if (prescaler >= 0x00 && prescaler <= 0x07) {
l |= OMAP_TIMER_CTRL_PRE;
l |= prescaler << 2;
+ } else {
+ if (prescaler != -1)
+ return -EINVAL;
}
omap_dm_timer_write_reg(timer, OMAP_TIMER_CTRL_REG, l);

--
2.15.1

2017-12-18 12:56:07

by Keerthy

[permalink] [raw]
Subject: Re: [PATCH v5 0/8] omap: dmtimer: Move driver out of plat-omap



On Monday 18 December 2017 04:46 PM, Ladislav Michl wrote:
> Keerthy,
>
> On Tue, Dec 12, 2017 at 11:42:09AM +0530, Keerthy wrote:
>> The series moves dmtimer out of plat-omap to drivers/clocksource.
>> The series also does a bunch of changes to pwm-omap-dmtimer code
>> to adapt to the driver migration and clean up plat specific
>> pdata-quirks and use the dmtimer platform data.
>
> thanks for nice work. I'll send two more patches as a reply to this
> one. I'd be glad if you could make them part of your serie. One of
> them would be nice to have for pwm driver prescaler fix which will
> be send independently.
> Also, if that helps you can have my
> Tested-by: Ladislav Michl <[email protected]>
> on IGEPv2 (OMAP3430 based)

Thanks a bunch! :-). I will take the pwm driver fix along with mine.

should i also take the two patches which you sent:

[PATCH 1/2] clocksource: timer-dm: Make unexported functions static
[PATCH 2/2] clocksource: timer-dm: Check prescaler value

Also what about this:
[PATCH] pwm: omap-dmtimer: Fix frequency when using prescaler

Let me know. I plan to send v6 with your Tested-by.

Thanks,
Keerthy

>
>> Boot tested on DRA7-EVM and AM437X-GP-EVM.
>
> I guess PWM driver was not tested, right? See comment to PATCH 7/8.
>
>> Compile tested omap1_defconfig.

Yes! Thanks for your feedback.

>
> Thank you,
> ladis
>

2017-12-18 12:56:30

by Keerthy

[permalink] [raw]
Subject: Re: [PATCH v5 7/8] pwm: pwm-omap-dmtimer: Adapt driver to utilize dmtimer pdata ops



On Monday 18 December 2017 03:01 PM, Ladislav Michl wrote:
> Keerthy,
>
> On Tue, Dec 12, 2017 at 11:42:16AM +0530, Keerthy wrote:
>> Adapt driver to utilize dmtimer pdata ops instead of pdata-quirks.
>>
>> Signed-off-by: Keerthy <[email protected]>
>> ---
>>
>> Changes in v4:
>>
>> * Switched to dev_get_platdata.
>
> Where do you expect dev.platform_data to be set? PWM driver is failing
> with:
> omap-dmtimer-pwm dmtimer-pwm: dmtimer pdata structure NULL
> omap-dmtimer-pwm: probe of dmtimer-pwm failed with error -22
>
> Which I fixed with patch bellow, to be able to test your patchset.

Thanks! I will make the below patch part of my series.

>
> Also I'm running a bit out of time, so I'll send few clean up
> patches and event capture code to get some feedback early.
>
> Regards,
> ladis
>
> diff --git a/drivers/clocksource/timer-dm.c b/drivers/clocksource/timer-dm.c
> index 39be39e6a8dd..d3d8a49cae0d 100644
> --- a/drivers/clocksource/timer-dm.c
> +++ b/drivers/clocksource/timer-dm.c
> @@ -773,6 +773,7 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
> dev_err(dev, "%s: no platform data.\n", __func__);
> return -ENODEV;
> }
> + dev->platform_data = pdata;
>
> irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> if (unlikely(!irq)) {
>
>>
>> Changes in v3:
>>
>> * Used of_find_platdata_by_node function to fetch platform
>> data for timer node.
>>
>> drivers/pwm/pwm-omap-dmtimer.c | 39 ++++++++++++++++++++++-----------------
>> 1 file changed, 22 insertions(+), 17 deletions(-)

2017-12-18 13:14:14

by Ladislav Michl

[permalink] [raw]
Subject: Re: [PATCH v5 0/8] omap: dmtimer: Move driver out of plat-omap

On Mon, Dec 18, 2017 at 06:24:42PM +0530, Keerthy wrote:
> On Monday 18 December 2017 04:46 PM, Ladislav Michl wrote:
> > Keerthy,
> >
> > On Tue, Dec 12, 2017 at 11:42:09AM +0530, Keerthy wrote:
> >> The series moves dmtimer out of plat-omap to drivers/clocksource.
> >> The series also does a bunch of changes to pwm-omap-dmtimer code
> >> to adapt to the driver migration and clean up plat specific
> >> pdata-quirks and use the dmtimer platform data.
> >
> > thanks for nice work. I'll send two more patches as a reply to this
> > one. I'd be glad if you could make them part of your serie. One of
> > them would be nice to have for pwm driver prescaler fix which will
> > be send independently.
> > Also, if that helps you can have my
> > Tested-by: Ladislav Michl <[email protected]>
> > on IGEPv2 (OMAP3430 based)
>
> Thanks a bunch! :-). I will take the pwm driver fix along with mine.
>
> should i also take the two patches which you sent:
>
> [PATCH 1/2] clocksource: timer-dm: Make unexported functions static
> [PATCH 2/2] clocksource: timer-dm: Check prescaler value

It would be great, if you could add those above to your serie.

> Also what about this:
> [PATCH] pwm: omap-dmtimer: Fix frequency when using prescaler

This one is independent of your serie, just "[PATCH 2/2] clocksource:
timer-dm: Check prescaler value" makes it a bit more error prone,
but there is no compile nor runtime dependency.

Perhaps it could me merged directly via pwm tree after some review?

I have few more patches to add event capture support, based on top
of those already sent, which I'll send separately.

> Let me know. I plan to send v6 with your Tested-by.
>
> Thanks,
> Keerthy

Thank you,
ladis

2017-12-19 04:59:26

by Keerthy

[permalink] [raw]
Subject: Re: [PATCH v5 7/8] pwm: pwm-omap-dmtimer: Adapt driver to utilize dmtimer pdata ops



On Monday 18 December 2017 06:25 PM, Keerthy wrote:
>
>
> On Monday 18 December 2017 03:01 PM, Ladislav Michl wrote:
>> Keerthy,
>>
>> On Tue, Dec 12, 2017 at 11:42:16AM +0530, Keerthy wrote:
>>> Adapt driver to utilize dmtimer pdata ops instead of pdata-quirks.
>>>
>>> Signed-off-by: Keerthy <[email protected]>
>>> ---
>>>
>>> Changes in v4:
>>>
>>> * Switched to dev_get_platdata.
>>
>> Where do you expect dev.platform_data to be set? PWM driver is failing
>> with:
>> omap-dmtimer-pwm dmtimer-pwm: dmtimer pdata structure NULL
>> omap-dmtimer-pwm: probe of dmtimer-pwm failed with error -22
>>
>> Which I fixed with patch bellow, to be able to test your patchset.
>
> Thanks! I will make the below patch part of my series.
>
>>
>> Also I'm running a bit out of time, so I'll send few clean up
>> patches and event capture code to get some feedback early.
>>
>> Regards,
>> ladis
>>
>> diff --git a/drivers/clocksource/timer-dm.c b/drivers/clocksource/timer-dm.c
>> index 39be39e6a8dd..d3d8a49cae0d 100644
>> --- a/drivers/clocksource/timer-dm.c
>> +++ b/drivers/clocksource/timer-dm.c
>> @@ -773,6 +773,7 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
>> dev_err(dev, "%s: no platform data.\n", __func__);
>> return -ENODEV;
>> }
>> + dev->platform_data = pdata;

drivers/clocksource/timer-dm.c: In function 'omap_dm_timer_probe':
drivers/clocksource/timer-dm.c:744:21: warning: assignment discards
'const' qualifier from pointer target type

This cannot be done as we are assigning a const pointer to a non-const
pointer.

I will figure out a different way for this fix.

>>
>> irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>> if (unlikely(!irq)) {
>>
>>>
>>> Changes in v3:
>>>
>>> * Used of_find_platdata_by_node function to fetch platform
>>> data for timer node.
>>>
>>> drivers/pwm/pwm-omap-dmtimer.c | 39 ++++++++++++++++++++++-----------------
>>> 1 file changed, 22 insertions(+), 17 deletions(-)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2017-12-19 08:26:44

by Keerthy

[permalink] [raw]
Subject: Re: [PATCH v5 7/8] pwm: pwm-omap-dmtimer: Adapt driver to utilize dmtimer pdata ops



On Tuesday 19 December 2017 10:28 AM, Keerthy wrote:
>
>
> On Monday 18 December 2017 06:25 PM, Keerthy wrote:
>>
>>
>> On Monday 18 December 2017 03:01 PM, Ladislav Michl wrote:
>>> Keerthy,
>>>
>>> On Tue, Dec 12, 2017 at 11:42:16AM +0530, Keerthy wrote:
>>>> Adapt driver to utilize dmtimer pdata ops instead of pdata-quirks.
>>>>
>>>> Signed-off-by: Keerthy <[email protected]>
>>>> ---
>>>>
>>>> Changes in v4:
>>>>
>>>> * Switched to dev_get_platdata.
>>>
>>> Where do you expect dev.platform_data to be set? PWM driver is failing
>>> with:
>>> omap-dmtimer-pwm dmtimer-pwm: dmtimer pdata structure NULL
>>> omap-dmtimer-pwm: probe of dmtimer-pwm failed with error -22
>>>
>>> Which I fixed with patch bellow, to be able to test your patchset.
>>
>> Thanks! I will make the below patch part of my series.
>>
>>>
>>> Also I'm running a bit out of time, so I'll send few clean up
>>> patches and event capture code to get some feedback early.
>>>
>>> Regards,
>>> ladis
>>>
>>> diff --git a/drivers/clocksource/timer-dm.c b/drivers/clocksource/timer-dm.c
>>> index 39be39e6a8dd..d3d8a49cae0d 100644
>>> --- a/drivers/clocksource/timer-dm.c
>>> +++ b/drivers/clocksource/timer-dm.c
>>> @@ -773,6 +773,7 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
>>> dev_err(dev, "%s: no platform data.\n", __func__);
>>> return -ENODEV;
>>> }
>>> + dev->platform_data = pdata;
>
> drivers/clocksource/timer-dm.c: In function 'omap_dm_timer_probe':
> drivers/clocksource/timer-dm.c:744:21: warning: assignment discards
> 'const' qualifier from pointer target type
>
> This cannot be done as we are assigning a const pointer to a non-const
> pointer.
>
> I will figure out a different way for this fix.

Ladis,

I fixed that:

diff --git a/drivers/clocksource/timer-dm.c b/drivers/clocksource/timer-dm.c
index 1cbd954..e58f555 100644
--- a/drivers/clocksource/timer-dm.c
+++ b/drivers/clocksource/timer-dm.c
@@ -807,17 +807,21 @@ static int omap_dm_timer_probe(struct
platform_device *pdev)
struct resource *mem, *irq;
struct device *dev = &pdev->dev;
const struct of_device_id *match;
- const struct dmtimer_platform_data *pdata;
+ struct dmtimer_platform_data *pdata;
int ret;

match = of_match_device(of_match_ptr(omap_timer_match), dev);
- pdata = match ? match->data : dev->platform_data;
+ pdata = match ? (struct dmtimer_platform_data *)match->data :
+ dev->platform_data;

if (!pdata && !dev->of_node) {
dev_err(dev, "%s: no platform data.\n", __func__);
return -ENODEV;
}

+ if (!dev->platform_data)
+ dev->platform_data = pdata;
+
irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
if (unlikely(!irq)) {
dev_err(dev, "%s: no IRQ resource.\n", __func__);
@@ -946,7 +950,7 @@ static int omap_dm_timer_remove(struct
platform_device *pdev)
.write_status = omap_dm_timer_write_status,
};

-static const struct dmtimer_platform_data omap3plus_pdata = {
+static struct dmtimer_platform_data omap3plus_pdata = {
.timer_errata = OMAP_TIMER_ERRATA_I103_I767,
.timer_ops = &dmtimer_ops,
};

Can you check at your end if this works for you?

>
>>>
>>> irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>>> if (unlikely(!irq)) {
>>>
>>>>
>>>> Changes in v3:
>>>>
>>>> * Used of_find_platdata_by_node function to fetch platform
>>>> data for timer node.
>>>>
>>>> drivers/pwm/pwm-omap-dmtimer.c | 39 ++++++++++++++++++++++-----------------
>>>> 1 file changed, 22 insertions(+), 17 deletions(-)
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>

2017-12-19 08:31:54

by Keerthy

[permalink] [raw]
Subject: Re: [PATCH 2/2] clocksource: timer-dm: Check prescaler value



On Monday 18 December 2017 05:01 PM, Ladislav Michl wrote:
> Invalid prescaler value is silently ignored. Fix that
> by returning -EINVAL in such case. As invalid value
> disabled use of the prescaler, use -1 explicitely for
> that purpose.

Thanks. I will post this as part of my migration series.

>
> Signed-off-by: Ladislav Michl <[email protected]>
> ---
> drivers/clocksource/timer-dm.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/clocksource/timer-dm.c b/drivers/clocksource/timer-dm.c
> index ec3a28c90c70..95cd98be8541 100644
> --- a/drivers/clocksource/timer-dm.c
> +++ b/drivers/clocksource/timer-dm.c
> @@ -609,6 +609,9 @@ static int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer,
> if (prescaler >= 0x00 && prescaler <= 0x07) {
> l |= OMAP_TIMER_CTRL_PRE;
> l |= prescaler << 2;
> + } else {
> + if (prescaler != -1)
> + return -EINVAL;
> }
> omap_dm_timer_write_reg(timer, OMAP_TIMER_CTRL_REG, l);
>
>

2017-12-19 08:36:27

by Keerthy

[permalink] [raw]
Subject: Re: [PATCH 1/2] clocksource: timer-dm: Make unexported functions static



On Monday 18 December 2017 05:00 PM, Ladislav Michl wrote:
> As dmtimer no longer exports functions, there is no point
> to have them non-static. Also delete those not used anywhere.

I will refrain from this patch at the moment. I have plans of another
series to do some more cleanups. For now want to keep the changes minimal.

Some of the functions that you are removing in this patch might be
needed and can be added for ops in future.
So i will let this be separate patch.

Thanks,
Keerthy

>
> Signed-off-by: Ladislav Michl <[email protected]>
> ---
> drivers/clocksource/timer-dm.c | 125 ++++++++---------------------------------
> include/clocksource/dmtimer.h | 26 ---------
> 2 files changed, 23 insertions(+), 128 deletions(-)
>
> diff --git a/drivers/clocksource/timer-dm.c b/drivers/clocksource/timer-dm.c
> index 392978ccd8a6..ec3a28c90c70 100644
> --- a/drivers/clocksource/timer-dm.c
> +++ b/drivers/clocksource/timer-dm.c
> @@ -204,16 +204,6 @@ static inline u32 omap_dm_timer_reserved_systimer(int id)
> return (omap_reserved_systimers & (1 << (id - 1))) ? 1 : 0;
> }
>
> -int omap_dm_timer_reserve_systimer(int id)
> -{
> - if (omap_dm_timer_reserved_systimer(id))
> - return -ENODEV;
> -
> - omap_reserved_systimers |= (1 << (id - 1));
> -
> - return 0;
> -}
> -
> static struct omap_dm_timer *_omap_dm_timer_request(int req_type, void *data)
> {
> struct omap_dm_timer *timer = NULL, *t;
> @@ -298,16 +288,16 @@ static struct omap_dm_timer *_omap_dm_timer_request(int req_type, void *data)
> return timer;
> }
>
> -struct omap_dm_timer *omap_dm_timer_request(void)
> +static struct omap_dm_timer *omap_dm_timer_request(void)
> {
> return _omap_dm_timer_request(REQUEST_ANY, NULL);
> }
>
> -struct omap_dm_timer *omap_dm_timer_request_specific(int id)
> +static struct omap_dm_timer *omap_dm_timer_request_specific(int id)
> {
> /* Requesting timer by ID is not supported when device tree is used */
> if (of_have_populated_dt()) {
> - pr_warn("%s: Please use omap_dm_timer_request_by_cap/node()\n",
> + pr_warn("%s: Please use omap_dm_timer_request_by_node()\n",
> __func__);
> return NULL;
> }
> @@ -315,20 +305,6 @@ struct omap_dm_timer *omap_dm_timer_request_specific(int id)
> return _omap_dm_timer_request(REQUEST_BY_ID, &id);
> }
>
> -/**
> - * omap_dm_timer_request_by_cap - Request a timer by capability
> - * @cap: Bit mask of capabilities to match
> - *
> - * Find a timer based upon capabilities bit mask. Callers of this function
> - * should use the definitions found in the plat/dmtimer.h file under the
> - * comment "timer capabilities used in hwmod database". Returns pointer to
> - * timer handle on success and a NULL pointer on failure.
> - */
> -struct omap_dm_timer *omap_dm_timer_request_by_cap(u32 cap)
> -{
> - return _omap_dm_timer_request(REQUEST_BY_CAP, &cap);
> -}
> -
> /**
> * omap_dm_timer_request_by_node - Request a timer by device-tree node
> * @np: Pointer to device-tree timer node
> @@ -336,7 +312,7 @@ struct omap_dm_timer *omap_dm_timer_request_by_cap(u32 cap)
> * Request a timer based upon a device node pointer. Returns pointer to
> * timer handle on success and a NULL pointer on failure.
> */
> -struct omap_dm_timer *omap_dm_timer_request_by_node(struct device_node *np)
> +static struct omap_dm_timer *omap_dm_timer_request_by_node(struct device_node *np)
> {
> if (!np)
> return NULL;
> @@ -344,7 +320,7 @@ struct omap_dm_timer *omap_dm_timer_request_by_node(struct device_node *np)
> return _omap_dm_timer_request(REQUEST_BY_NODE, np);
> }
>
> -int omap_dm_timer_free(struct omap_dm_timer *timer)
> +static int omap_dm_timer_free(struct omap_dm_timer *timer)
> {
> if (unlikely(!timer))
> return -EINVAL;
> @@ -424,7 +400,7 @@ __u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)
>
> #else
>
> -struct clk *omap_dm_timer_get_fclk(struct omap_dm_timer *timer)
> +static struct clk *omap_dm_timer_get_fclk(struct omap_dm_timer *timer)
> {
> if (timer && !IS_ERR(timer->fclk))
> return timer->fclk;
> @@ -440,18 +416,7 @@ __u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)
>
> #endif
>
> -int omap_dm_timer_trigger(struct omap_dm_timer *timer)
> -{
> - if (unlikely(!timer || pm_runtime_suspended(&timer->pdev->dev))) {
> - pr_err("%s: timer not available or enabled.\n", __func__);
> - return -EINVAL;
> - }
> -
> - omap_dm_timer_write_reg(timer, OMAP_TIMER_TRIGGER_REG, 0);
> - return 0;
> -}
> -
> -int omap_dm_timer_start(struct omap_dm_timer *timer)
> +static int omap_dm_timer_start(struct omap_dm_timer *timer)
> {
> u32 l;
>
> @@ -471,7 +436,7 @@ int omap_dm_timer_start(struct omap_dm_timer *timer)
> return 0;
> }
>
> -int omap_dm_timer_stop(struct omap_dm_timer *timer)
> +static int omap_dm_timer_stop(struct omap_dm_timer *timer)
> {
> unsigned long rate = 0;
>
> @@ -556,8 +521,8 @@ int omap_dm_timer_set_source(struct omap_dm_timer *timer, int source)
> return ret;
> }
>
> -int omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload,
> - unsigned int load)
> +static int omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload,
> + unsigned int load)
> {
> u32 l;
>
> @@ -581,37 +546,8 @@ int omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload,
> return 0;
> }
>
> -/* Optimized set_load which removes costly spin wait in timer_start */
> -int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, int autoreload,
> - unsigned int load)
> -{
> - u32 l;
> -
> - if (unlikely(!timer))
> - return -EINVAL;
> -
> - omap_dm_timer_enable(timer);
> -
> - l = omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
> - if (autoreload) {
> - l |= OMAP_TIMER_CTRL_AR;
> - omap_dm_timer_write_reg(timer, OMAP_TIMER_LOAD_REG, load);
> - } else {
> - l &= ~OMAP_TIMER_CTRL_AR;
> - }
> - l |= OMAP_TIMER_CTRL_ST;
> -
> - __omap_dm_timer_load_start(timer, l, load, timer->posted);
> -
> - /* Save the context */
> - timer->context.tclr = l;
> - timer->context.tldr = load;
> - timer->context.tcrr = load;
> - return 0;
> -}
> -
> -int omap_dm_timer_set_match(struct omap_dm_timer *timer, int enable,
> - unsigned int match)
> +static int omap_dm_timer_set_match(struct omap_dm_timer *timer, int enable,
> + unsigned int match)
> {
> u32 l;
>
> @@ -634,8 +570,8 @@ int omap_dm_timer_set_match(struct omap_dm_timer *timer, int enable,
> return 0;
> }
>
> -int omap_dm_timer_set_pwm(struct omap_dm_timer *timer, int def_on,
> - int toggle, int trigger)
> +static int omap_dm_timer_set_pwm(struct omap_dm_timer *timer, int def_on,
> + int toggle, int trigger)
> {
> u32 l;
>
> @@ -659,7 +595,8 @@ int omap_dm_timer_set_pwm(struct omap_dm_timer *timer, int def_on,
> return 0;
> }
>
> -int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer, int prescaler)
> +static int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer,
> + int prescaler)
> {
> u32 l;
>
> @@ -681,8 +618,8 @@ int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer, int prescaler)
> return 0;
> }
>
> -int omap_dm_timer_set_int_enable(struct omap_dm_timer *timer,
> - unsigned int value)
> +static int omap_dm_timer_set_int_enable(struct omap_dm_timer *timer,
> + unsigned int value)
> {
> if (unlikely(!timer))
> return -EINVAL;
> @@ -704,7 +641,7 @@ int omap_dm_timer_set_int_enable(struct omap_dm_timer *timer,
> *
> * Disables the specified timer interrupts for a timer.
> */
> -int omap_dm_timer_set_int_disable(struct omap_dm_timer *timer, u32 mask)
> +static int omap_dm_timer_set_int_disable(struct omap_dm_timer *timer, u32 mask)
> {
> u32 l = mask;
>
> @@ -727,7 +664,7 @@ int omap_dm_timer_set_int_disable(struct omap_dm_timer *timer, u32 mask)
> return 0;
> }
>
> -unsigned int omap_dm_timer_read_status(struct omap_dm_timer *timer)
> +static unsigned int omap_dm_timer_read_status(struct omap_dm_timer *timer)
> {
> unsigned int l;
>
> @@ -741,7 +678,7 @@ unsigned int omap_dm_timer_read_status(struct omap_dm_timer *timer)
> return l;
> }
>
> -int omap_dm_timer_write_status(struct omap_dm_timer *timer, unsigned int value)
> +static int omap_dm_timer_write_status(struct omap_dm_timer *timer, unsigned int value)
> {
> if (unlikely(!timer || pm_runtime_suspended(&timer->pdev->dev)))
> return -EINVAL;
> @@ -751,7 +688,7 @@ int omap_dm_timer_write_status(struct omap_dm_timer *timer, unsigned int value)
> return 0;
> }
>
> -unsigned int omap_dm_timer_read_counter(struct omap_dm_timer *timer)
> +static unsigned int omap_dm_timer_read_counter(struct omap_dm_timer *timer)
> {
> if (unlikely(!timer || pm_runtime_suspended(&timer->pdev->dev))) {
> pr_err("%s: timer not iavailable or enabled.\n", __func__);
> @@ -761,7 +698,7 @@ unsigned int omap_dm_timer_read_counter(struct omap_dm_timer *timer)
> return __omap_dm_timer_read_counter(timer, timer->posted);
> }
>
> -int omap_dm_timer_write_counter(struct omap_dm_timer *timer, unsigned int value)
> +static int omap_dm_timer_write_counter(struct omap_dm_timer *timer, unsigned int value)
> {
> if (unlikely(!timer || pm_runtime_suspended(&timer->pdev->dev))) {
> pr_err("%s: timer not available or enabled.\n", __func__);
> @@ -775,22 +712,6 @@ int omap_dm_timer_write_counter(struct omap_dm_timer *timer, unsigned int value)
> return 0;
> }
>
> -int omap_dm_timers_active(void)
> -{
> - struct omap_dm_timer *timer;
> -
> - list_for_each_entry(timer, &omap_timer_list, node) {
> - if (!timer->reserved)
> - continue;
> -
> - if (omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG) &
> - OMAP_TIMER_CTRL_ST) {
> - return 1;
> - }
> - }
> - return 0;
> -}
> -
> static const struct of_device_id omap_timer_match[];
>
> /**
> diff --git a/include/clocksource/dmtimer.h b/include/clocksource/dmtimer.h
> index 862ad62dab9d..63d6ec0747d9 100644
> --- a/include/clocksource/dmtimer.h
> +++ b/include/clocksource/dmtimer.h
> @@ -124,40 +124,14 @@ struct omap_dm_timer {
> struct list_head node;
> };
>
> -int omap_dm_timer_reserve_systimer(int id);
> -struct omap_dm_timer *omap_dm_timer_request(void);
> -struct omap_dm_timer *omap_dm_timer_request_specific(int timer_id);
> -struct omap_dm_timer *omap_dm_timer_request_by_cap(u32 cap);
> -struct omap_dm_timer *omap_dm_timer_request_by_node(struct device_node *np);
> -int omap_dm_timer_free(struct omap_dm_timer *timer);
> void omap_dm_timer_enable(struct omap_dm_timer *timer);
> void omap_dm_timer_disable(struct omap_dm_timer *timer);
>
> int omap_dm_timer_get_irq(struct omap_dm_timer *timer);
>
> u32 omap_dm_timer_modify_idlect_mask(u32 inputmask);
> -struct clk *omap_dm_timer_get_fclk(struct omap_dm_timer *timer);
> -
> -int omap_dm_timer_trigger(struct omap_dm_timer *timer);
> -int omap_dm_timer_start(struct omap_dm_timer *timer);
> -int omap_dm_timer_stop(struct omap_dm_timer *timer);
>
> int omap_dm_timer_set_source(struct omap_dm_timer *timer, int source);
> -int omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload, unsigned int value);
> -int omap_dm_timer_set_load_start(struct omap_dm_timer *timer, int autoreload, unsigned int value);
> -int omap_dm_timer_set_match(struct omap_dm_timer *timer, int enable, unsigned int match);
> -int omap_dm_timer_set_pwm(struct omap_dm_timer *timer, int def_on, int toggle, int trigger);
> -int omap_dm_timer_set_prescaler(struct omap_dm_timer *timer, int prescaler);
> -
> -int omap_dm_timer_set_int_enable(struct omap_dm_timer *timer, unsigned int value);
> -int omap_dm_timer_set_int_disable(struct omap_dm_timer *timer, u32 mask);
> -
> -unsigned int omap_dm_timer_read_status(struct omap_dm_timer *timer);
> -int omap_dm_timer_write_status(struct omap_dm_timer *timer, unsigned int value);
> -unsigned int omap_dm_timer_read_counter(struct omap_dm_timer *timer);
> -int omap_dm_timer_write_counter(struct omap_dm_timer *timer, unsigned int value);
> -
> -int omap_dm_timers_active(void);
>
> /*
> * Do not use the defines below, they are not needed. They should be only
>

2017-12-19 15:21:53

by Ladislav Michl

[permalink] [raw]
Subject: Re: [PATCH v5 7/8] pwm: pwm-omap-dmtimer: Adapt driver to utilize dmtimer pdata ops

On Tue, Dec 19, 2017 at 01:55:48PM +0530, Keerthy wrote:
> On Tuesday 19 December 2017 10:28 AM, Keerthy wrote:
> > On Monday 18 December 2017 06:25 PM, Keerthy wrote:
> >> On Monday 18 December 2017 03:01 PM, Ladislav Michl wrote:
> >>> Keerthy,
> >>>
> >>> On Tue, Dec 12, 2017 at 11:42:16AM +0530, Keerthy wrote:
> >>>> Adapt driver to utilize dmtimer pdata ops instead of pdata-quirks.
> >>>>
> >>>> Signed-off-by: Keerthy <[email protected]>
> >>>> ---
> >>>>
> >>>> Changes in v4:
> >>>>
> >>>> * Switched to dev_get_platdata.
> >>>
> >>> Where do you expect dev.platform_data to be set? PWM driver is failing
> >>> with:
> >>> omap-dmtimer-pwm dmtimer-pwm: dmtimer pdata structure NULL
> >>> omap-dmtimer-pwm: probe of dmtimer-pwm failed with error -22
> >>>
> >>> Which I fixed with patch bellow, to be able to test your patchset.
> >>
> >> Thanks! I will make the below patch part of my series.
> >>
> >>>
> >>> Also I'm running a bit out of time, so I'll send few clean up
> >>> patches and event capture code to get some feedback early.
> >>>
> >>> Regards,
> >>> ladis
> >>>
> >>> diff --git a/drivers/clocksource/timer-dm.c b/drivers/clocksource/timer-dm.c
> >>> index 39be39e6a8dd..d3d8a49cae0d 100644
> >>> --- a/drivers/clocksource/timer-dm.c
> >>> +++ b/drivers/clocksource/timer-dm.c
> >>> @@ -773,6 +773,7 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
> >>> dev_err(dev, "%s: no platform data.\n", __func__);
> >>> return -ENODEV;
> >>> }
> >>> + dev->platform_data = pdata;
> >
> > drivers/clocksource/timer-dm.c: In function 'omap_dm_timer_probe':
> > drivers/clocksource/timer-dm.c:744:21: warning: assignment discards
> > 'const' qualifier from pointer target type
> >
> > This cannot be done as we are assigning a const pointer to a non-const
> > pointer.

Oh, I didn't even assume it as proper fix, just to show what is missing :)

But technically 'struct dmtimer_platform_data *pdata' is a constant which
should not be changed. Also look how all that of_populate chain works -
at the end const pointer is assigned to void* platform_data by simple
(void *) overcast.

> > I will figure out a different way for this fix.
>
> Ladis,
>
> I fixed that:
>
> diff --git a/drivers/clocksource/timer-dm.c b/drivers/clocksource/timer-dm.c
> index 1cbd954..e58f555 100644
> --- a/drivers/clocksource/timer-dm.c
> +++ b/drivers/clocksource/timer-dm.c
> @@ -807,17 +807,21 @@ static int omap_dm_timer_probe(struct
> platform_device *pdev)
> struct resource *mem, *irq;
> struct device *dev = &pdev->dev;
> const struct of_device_id *match;
> - const struct dmtimer_platform_data *pdata;
> + struct dmtimer_platform_data *pdata;
> int ret;
>
> match = of_match_device(of_match_ptr(omap_timer_match), dev);
> - pdata = match ? match->data : dev->platform_data;
> + pdata = match ? (struct dmtimer_platform_data *)match->data :
> + dev->platform_data;

All that seems needlesly complicated, what about patch bellow?

> if (!pdata && !dev->of_node) {
> dev_err(dev, "%s: no platform data.\n", __func__);
> return -ENODEV;
> }
>
> + if (!dev->platform_data)
> + dev->platform_data = pdata;

Does the above condition bring us anything?

> irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> if (unlikely(!irq)) {
> dev_err(dev, "%s: no IRQ resource.\n", __func__);
> @@ -946,7 +950,7 @@ static int omap_dm_timer_remove(struct
> platform_device *pdev)
> .write_status = omap_dm_timer_write_status,
> };
>
> -static const struct dmtimer_platform_data omap3plus_pdata = {
> +static struct dmtimer_platform_data omap3plus_pdata = {
> .timer_errata = OMAP_TIMER_ERRATA_I103_I767,
> .timer_ops = &dmtimer_ops,
> };
>
> Can you check at your end if this works for you?

Note, it is untested as I ran out of time and will continue after New Year.

diff --git a/drivers/clocksource/timer-dm.c b/drivers/clocksource/timer-dm.c
index 1cbd95420914..85024f11773a 100644
--- a/drivers/clocksource/timer-dm.c
+++ b/drivers/clocksource/timer-dm.c
@@ -806,14 +806,16 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
struct omap_dm_timer *timer;
struct resource *mem, *irq;
struct device *dev = &pdev->dev;
- const struct of_device_id *match;
const struct dmtimer_platform_data *pdata;
int ret;

- match = of_match_device(of_match_ptr(omap_timer_match), dev);
- pdata = match ? match->data : dev->platform_data;
+ pdata = of_device_get_match_data(dev);
+ if (!pdata)
+ pdata = dev_get_platdata(dev);
+ else
+ dev->platform_data = (void *) pdata;

- if (!pdata && !dev->of_node) {
+ if (!pdata) {
dev_err(dev, "%s: no platform data.\n", __func__);
return -ENODEV;
}

2017-12-20 04:45:14

by Keerthy

[permalink] [raw]
Subject: Re: [PATCH v5 7/8] pwm: pwm-omap-dmtimer: Adapt driver to utilize dmtimer pdata ops



On Tuesday 19 December 2017 08:51 PM, Ladislav Michl wrote:
> On Tue, Dec 19, 2017 at 01:55:48PM +0530, Keerthy wrote:
>> On Tuesday 19 December 2017 10:28 AM, Keerthy wrote:
>>> On Monday 18 December 2017 06:25 PM, Keerthy wrote:
>>>> On Monday 18 December 2017 03:01 PM, Ladislav Michl wrote:
>>>>> Keerthy,
>>>>>
>>>>> On Tue, Dec 12, 2017 at 11:42:16AM +0530, Keerthy wrote:
>>>>>> Adapt driver to utilize dmtimer pdata ops instead of pdata-quirks.
>>>>>>
>>>>>> Signed-off-by: Keerthy <[email protected]>
>>>>>> ---
>>>>>>
>>>>>> Changes in v4:
>>>>>>
>>>>>> * Switched to dev_get_platdata.
>>>>>
>>>>> Where do you expect dev.platform_data to be set? PWM driver is failing
>>>>> with:
>>>>> omap-dmtimer-pwm dmtimer-pwm: dmtimer pdata structure NULL
>>>>> omap-dmtimer-pwm: probe of dmtimer-pwm failed with error -22
>>>>>
>>>>> Which I fixed with patch bellow, to be able to test your patchset.
>>>>
>>>> Thanks! I will make the below patch part of my series.
>>>>
>>>>>
>>>>> Also I'm running a bit out of time, so I'll send few clean up
>>>>> patches and event capture code to get some feedback early.
>>>>>
>>>>> Regards,
>>>>> ladis
>>>>>
>>>>> diff --git a/drivers/clocksource/timer-dm.c b/drivers/clocksource/timer-dm.c
>>>>> index 39be39e6a8dd..d3d8a49cae0d 100644
>>>>> --- a/drivers/clocksource/timer-dm.c
>>>>> +++ b/drivers/clocksource/timer-dm.c
>>>>> @@ -773,6 +773,7 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
>>>>> dev_err(dev, "%s: no platform data.\n", __func__);
>>>>> return -ENODEV;
>>>>> }
>>>>> + dev->platform_data = pdata;
>>>
>>> drivers/clocksource/timer-dm.c: In function 'omap_dm_timer_probe':
>>> drivers/clocksource/timer-dm.c:744:21: warning: assignment discards
>>> 'const' qualifier from pointer target type
>>>
>>> This cannot be done as we are assigning a const pointer to a non-const
>>> pointer.
>
> Oh, I didn't even assume it as proper fix, just to show what is missing :)
>
> But technically 'struct dmtimer_platform_data *pdata' is a constant which
> should not be changed. Also look how all that of_populate chain works -
> at the end const pointer is assigned to void* platform_data by simple
> (void *) overcast.
>
>>> I will figure out a different way for this fix.
>>
>> Ladis,
>>
>> I fixed that:
>>
>> diff --git a/drivers/clocksource/timer-dm.c b/drivers/clocksource/timer-dm.c
>> index 1cbd954..e58f555 100644
>> --- a/drivers/clocksource/timer-dm.c
>> +++ b/drivers/clocksource/timer-dm.c
>> @@ -807,17 +807,21 @@ static int omap_dm_timer_probe(struct
>> platform_device *pdev)
>> struct resource *mem, *irq;
>> struct device *dev = &pdev->dev;
>> const struct of_device_id *match;
>> - const struct dmtimer_platform_data *pdata;
>> + struct dmtimer_platform_data *pdata;
>> int ret;
>>
>> match = of_match_device(of_match_ptr(omap_timer_match), dev);
>> - pdata = match ? match->data : dev->platform_data;
>> + pdata = match ? (struct dmtimer_platform_data *)match->data :
>> + dev->platform_data;
>
> All that seems needlesly complicated, what about patch bellow?
>
>> if (!pdata && !dev->of_node) {
>> dev_err(dev, "%s: no platform data.\n", __func__);
>> return -ENODEV;
>> }
>>
>> + if (!dev->platform_data)
>> + dev->platform_data = pdata;
>
> Does the above condition bring us anything?

That was to avoid assigning the same thing.

>
>> irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>> if (unlikely(!irq)) {
>> dev_err(dev, "%s: no IRQ resource.\n", __func__);
>> @@ -946,7 +950,7 @@ static int omap_dm_timer_remove(struct
>> platform_device *pdev)
>> .write_status = omap_dm_timer_write_status,
>> };
>>
>> -static const struct dmtimer_platform_data omap3plus_pdata = {
>> +static struct dmtimer_platform_data omap3plus_pdata = {
>> .timer_errata = OMAP_TIMER_ERRATA_I103_I767,
>> .timer_ops = &dmtimer_ops,
>> };
>>
>> Can you check at your end if this works for you?
>
> Note, it is untested as I ran out of time and will continue after New Year.
>
> diff --git a/drivers/clocksource/timer-dm.c b/drivers/clocksource/timer-dm.c
> index 1cbd95420914..85024f11773a 100644
> --- a/drivers/clocksource/timer-dm.c
> +++ b/drivers/clocksource/timer-dm.c
> @@ -806,14 +806,16 @@ static int omap_dm_timer_probe(struct platform_device *pdev)
> struct omap_dm_timer *timer;
> struct resource *mem, *irq;
> struct device *dev = &pdev->dev;
> - const struct of_device_id *match;
> const struct dmtimer_platform_data *pdata;
> int ret;
>
> - match = of_match_device(of_match_ptr(omap_timer_match), dev);
> - pdata = match ? match->data : dev->platform_data;
> + pdata = of_device_get_match_data(dev);
> + if (!pdata)
> + pdata = dev_get_platdata(dev);
> + else
> + dev->platform_data = (void *) pdata;
>
> - if (!pdata && !dev->of_node) {
> + if (!pdata) {
> dev_err(dev, "%s: no platform data.\n", __func__);
> return -ENODEV;
> }

Ladis,

I have tested this on AM437 for dmtimer only. But i have checked that
pdata gets neatly assigned to dev->platform_data. So i believe that was
what was lacking. I will pick the above patch from you and post v6 with
your Tested-by as the pdata hooking was the only missing link for pwm.

Thanks for the patch once again.

Regards,
Keerthy
>