Re: [PATCH v5 08/14] ARM: OMAP2+: gpmc: bool type timing helper
On 06/11/2012 09:27 AM, Afzal Mohammed wrote:
> Some of the timing configuration like extra delay
> has bool type configurations. Provide a helper so
> that these too can be configured in Kernel.
>
> Signed-off-by: Afzal Mohammed
> ---
> arch/arm/mach-omap2/gpmc.c | 55
>
> 1 file changed, 55 insertions(+)
>
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index e60076e3..65052f8 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -68,6 +68,13 @@
> #define GPMC_ECC_CTRL_ECCREG80x008
> #define GPMC_ECC_CTRL_ECCREG90x009
>
> +#define GPMC_CONFIG2_CSEXTRADELAY BIT(7)
> +#define GPMC_CONFIG3_ADVEXTRADELAY BIT(7)
> +#define GPMC_CONFIG4_OEEXTRADELAY BIT(7)
> +#define GPMC_CONFIG4_WEEXTRADELAY BIT(23)
> +#define GPMC_CONFIG6_CYCLE2CYCLEDIFFCSENBIT(6)
> +#define GPMC_CONFIG6_CYCLE2CYCLESAMECSENBIT(7)
> +
> #define GPMC_CS0_OFFSET 0x60
> #define GPMC_CS_SIZE 0x30
>
> @@ -960,6 +967,54 @@ static void gpmc_setup_cs_config(unsigned cs, unsigned
> conf)
> gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, l);
> }
>
> +static void gpmc_cs_misc_timings(int cs, const struct gpmc_misc_timings *p)
> +{
> + u32 l;
> +
> + l = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
> + if (p->time_para_granularity)
> + l |= GPMC_CONFIG1_TIME_PARA_GRAN;
> + else
> + l &= ~GPMC_CONFIG1_TIME_PARA_GRAN;
> + gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, l);
> +
> + l = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG2);
> + if (p->cs_extra_delay)
> + l |= GPMC_CONFIG2_CSEXTRADELAY;
> + else
> + l &= ~GPMC_CONFIG2_CSEXTRADELAY;
> + gpmc_cs_write_reg(cs, GPMC_CS_CONFIG2, l);
> +
> + l = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG3);
> + if (p->adv_extra_delay)
> + l |= GPMC_CONFIG3_ADVEXTRADELAY;
> + else
> + l &= ~GPMC_CONFIG3_ADVEXTRADELAY;
> + gpmc_cs_write_reg(cs, GPMC_CS_CONFIG3, l);
> +
> + l = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG4);
> + if (p->oe_extra_delay)
> + l |= GPMC_CONFIG4_OEEXTRADELAY;gpmc_cs_misc_timings
> + else
> + l &= ~GPMC_CONFIG4_OEEXTRADELAY;
> + if (p->we_extra_delay)
> + l |= GPMC_CONFIG4_WEEXTRADELAY;
> + else
> + l &= ~GPMC_CONFIG4_WEEXTRADELAY;
> + gpmc_cs_write_reg(cs, GPMC_CS_CONFIG4, l);
> +
> + l = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG6);
> + if (p->cycle2cyclesamecsen)
> + l |= GPMC_CONFIG6_CYCLE2CYCLESAMECSEN;
> + else
> + l &= ~GPMC_CONFIG6_CYCLE2CYCLESAMECSEN;
> + if (p->cycle2cyclediffcsen)
> + l |= GPMC_CONFIG6_CYCLE2CYCLEDIFFCSEN;
> + else
> + l &= ~GPMC_CONFIG6_CYCLE2CYCLEDIFFCSEN;
> + gpmc_cs_write_reg(cs, GPMC_CS_CONFIG6, l);
> +}
How about adding a sub-function like ...
static inline
void _gpmc_cs_misc_timings(int cs, int reg, int flag, int bit)
{
if (flag)
gpmc_set_one_timing(cs, reg, bit, bit, 1);
else
gpmc_set_one_timing(cs, reg, bit, bit, 0);
}
Or maybe make it into a generic set/clear bit function. Should reduce
overall lines of code.
Cheers
Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 09/14] ARM: OMAP2+: gpmc: holler if no configuration
On 06/11/2012 09:27 AM, Afzal Mohammed wrote:
> Some of the GPMC peripherals depend on bootloader to do the
> configuration. This facility is deprecated, notify user
> about the present GPMC settings & inform that that relying
> on bootloader for GPMC setting is deprecated.
Nit, "holler" is slang. Just say WARN.
Jon
> Signed-off-by: Afzal Mohammed
> ---
> arch/arm/mach-omap2/gpmc.c | 104
>
> 1 file changed, 104 insertions(+)
>
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index 65052f8..5a6f708 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -1058,6 +1058,110 @@ static void gpmc_cs_set_register_timings(int cs,
> const struct gpmc_timings *t)
> }
> }
>
> +static void gpmc_print_cs_config(int cs)
> +{
> + u32 l = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
> +
> + dev_warn(gpmc_dev, "GPMC CS%d depends on bootloader for config\n", cs);
> + dev_warn(gpmc_dev, "Please update it in Kernel ASAP to prevent losing
> support for this peripheral\n");
> + dev_warn(gpmc_dev, "Bootloader dependency for GPMC configuration is
> deprecated\n");
> +
> + dev_warn(gpmc_dev, "muxadddata: %s\n",
> + l & GPMC_CONFIG1_MUXADDDATA ? "yes" : "no");
> + dev_warn(gpmc_dev, "writetypesync: %s\n",
> + l & GPMC_CONFIG1_WRITETYPE_SYNC ? "yes" : "no");
> + dev_warn(gpmc_dev, "writemultiple: %s\n",
> + l & GPMC_CONFIG1_WRITEMULTIPLE_SUPP ? "yes" : "no");
> + dev_warn(gpmc_dev, "readtypesync: %s\n",
> + l & GPMC_CONFIG1_READTYPE_SYNC ? "yes" : "no");
> + dev_warn(gpmc_dev, "readmultiple: %s\n",
> + l & GPMC_CONFIG1_READMULTIPLE_SUPP ? "yes" : "no");
> + dev_warn(gpmc_dev, "wrapburst: %s\n",
> + l & GPMC_CONFIG1_WRAPBURST_SUPP ? "yes" : "no");
> + dev_warn(gpmc_dev, "devicetype: %s\n",
> + l & GPMC_CONFIG1_DEVICETYPE_NAND ? "nand" : "nor");
> + dev_warn(gpmc_dev, "devicesize: %d\n",
> + l & GPMC_CONFIG1_DEVICESIZE_16 ? 16 : 8);
> + dev_warn(gpmc_dev, "pagelength: %d\n",
> + l & GPMC_CONFIG1_PAGE_LEN(~0) ?
> + (l & GPMC_CONFIG1_PAGE_LEN_16 ? 16 : 8) : 4);
> +}
> +static inline unsigned gpmc_get_one_timing(int cs, int reg, int start, int
> end)
> +{
> + u32 l = gpmc_cs_read_reg(cs, reg);
> + unsigned mask;
> +
> + mask = (1 << (end - start + 1)) - 1;
> + l &= (mask << start);
> + return l >>= start;
> +}
> +
> +static void gpmc_print_cs_timings(int cs)
> +{
> + dev_warn(gpmc_dev, "GPMC CS%d depends on bootloader for timing\n", cs);
> + dev_warn(gpmc_dev, "Please update it in Kernel ASAP to prevent losing
> support for this peripheral\n");
> + dev_warn(gpmc_dev, "Bootloader dependency for GPMC configuration is
> deprecated\n");
> +
> + dev_warn(gpmc_dev, "fclk period: %lups\n", gpmc_get_fclk_period());
> + dev_warn(gpmc_dev, "sync_clk: %u\n",
> + gpmc_get_one_timing(cs, GPMC_CS_CONFIG1, 0, 1));
> + dev_warn(gpmc_dev, "wait_monitoring: %u\n",
> + gpmc_get_one_timing(cs, GPMC_CS_CONFIG1, 18, 19));
> + dev_warn(gpmc_dev, "clk_activation: %u\n",
> + gpmc_get_one_timing(cs, GPMC_CS_CONFIG1, 25, 26));
> + dev_warn(gpmc_dev, "cs_on: %u\n",
> + gpmc_get_one_timing(cs, GPMC_CS_CONFIG2, 0, 3));
> + dev_warn(gpmc_dev, "cs_rd_off: %u\n",
> + gpmc_get_one_timing(cs, GPMC_CS_CONFIG2, 8, 12));
> + dev_warn(gpmc_dev, "cs_wr_off: %u\n",
> + gpmc_get_one_timing(cs, GPMC_CS_CONFIG2, 16, 20));
> + dev_warn(gpmc_dev, "adv_on: %u\n",
> + gpmc_get_one_timing(cs, GPMC_CS_CONFIG3, 0, 3));
> + dev_warn(gpmc_dev, "adv_rd_off: %u\n",
> + gpmc_get_one_timing(cs, GPMC_CS_CONFIG3, 8, 12));
> + dev_warn(gpmc_dev, "adv_wr_off: %u\n",
> + gpmc_get_one_timing(cs, GPMC_CS_CONFIG3, 16, 20));
> + dev_warn(gpmc_dev, "oe_on: %u\n",
> + gpmc_get_one_timing(cs, GPMC_CS_CONFIG4, 0, 3));
> + dev_warn(gpmc_dev, "oe_off: %u\n",
> + gpmc_get_one_timing(cs, GPMC_CS_CONFIG4, 8, 12));
> + dev_warn(gpmc_dev, "we_on: %u\n",
> + gpmc_get_one_timing(cs, GPMC_CS_CONFIG4, 16, 19));
> + dev_warn(gpmc_dev, "we_off: %u\n",
> + gpmc_get_one_timing(cs, GPMC_CS_CONFIG4, 24, 28));
> + dev_warn(gpmc_dev, "rd_cycle: %u\n",
> + gpmc_get_one_timing(cs, GPMC_CS_CONFIG5, 0, 4));
> + dev_warn(gpmc_dev, "wr_cycle: %u\n",
> + gpmc_get_one_timing(cs, GPMC_CS_CONFIG5, 8, 12));
> + dev_warn(gpmc_dev, "acess: %u\n",
> + gpmc_get_one_timing(cs, GPMC_CS_CONFIG5, 16, 20));
> + dev_warn(gpmc_dev, "page_burst_access: %u\n",
> +
Re: [PATCH v5 10/14] ARM: OMAP2+: gpmc: waitpin helper
On 06/11/2012 09:27 AM, Afzal Mohammed wrote:
> Helper for configuring waitpin. There are two parts to it;
> configuring at CS level and the other at device level.
> A device embedding multiple CS has been provided the
> capability to use same waitpin (different waitpins has not
> been supported as presently there are no GPMC peripherals
> doing so)
>
> Signed-off-by: Afzal Mohammed
> ---
> arch/arm/mach-omap2/gpmc.c | 122
>
> arch/arm/plat-omap/include/plat/gpmc.h |9 +++
> 2 files changed, 131 insertions(+)
>
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index 5a6f708..9073a8a 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -75,6 +75,8 @@
> #define GPMC_CONFIG6_CYCLE2CYCLEDIFFCSENBIT(6)
> #define GPMC_CONFIG6_CYCLE2CYCLESAMECSENBIT(7)
>
> +#define GPMC_CONFIG_WAITPIN_POLARITY_SHIFT 0x8
> +
> #define GPMC_CS0_OFFSET 0x60
> #define GPMC_CS_SIZE 0x30
>
> @@ -93,6 +95,19 @@
> */
> #define GPMC_NR_IRQ 2
>
> +enum {
> + GPMC_WAITPIN_IDX0,
> + GPMC_WAITPIN_IDX1,
> + GPMC_WAITPIN_IDX2,
> + GPMC_WAITPIN_IDX3,
> + GPMC_NR_WAITPIN
> +};
Max number of wait pins is 3 for omap4/5. I know that we discussed this
in the past, but are you not supporting these devices are the moment? I
know that you have not done the hwmod for these, but still I was hoping
that you would take these into account.
> +
> +enum {
> + LOW,
> + HIGH
> +};
To be honest, I don't see the point in either of the above enums when
you have the definitions at the bottom. Seems that one set of
definitions should be enough.
> struct gpmc_client_irq {
> unsignedirq;
> u32 bitmask;
> @@ -140,6 +155,8 @@ struct gpmc_peripheral {
> struct platform_device *pdev;
> };
>
> +static unsigned gpmc_waitpin_map;
> +
> static struct gpmc_client_irq gpmc_client_irq[GPMC_NR_IRQ];
> static struct irq_chip gpmc_irq_chip;
> static unsigned gpmc_irq_start;
> @@ -1162,6 +1179,62 @@ static void gpmc_print_cs_timings(int cs)
> gpmc_get_one_timing(cs, GPMC_CS_CONFIG6, 7, 7));
> }
>
> +static int gpmc_setup_cs_waitpin(struct gpmc_peripheral *g_per, unsigned cs,
> + unsigned conf)
> +{
> + unsigned idx;
> + bool polarity = 0;
> + u32 l = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG1);
> +
> + switch (conf & GPMC_WAITPIN_MASK) {
> + case GPMC_WAITPIN_0:
> + idx = GPMC_WAITPIN_IDX0;
For example, here you could have ...
idx = GPMC_WAITPIN_0 - 1;
> + break;
> + case GPMC_WAITPIN_1:
> + idx = GPMC_WAITPIN_IDX1;
> + break;
> + case GPMC_WAITPIN_2:
> + idx = GPMC_WAITPIN_IDX2;
> + break;
> + case GPMC_WAITPIN_3:
> + idx = GPMC_WAITPIN_IDX3;
> + break;
> + /* no waitpin */
> + case 0:
> + return 0;
> + break;
> + default:
> + dev_err(gpmc_dev, "multiple waitpins selected on CS:%u\n", cs);
> + return -EINVAL;
> + break;
> + }
> +
> + polarity = !!(conf & GPMC_WAITPIN_ACTIVE_HIGH);
> +
> + if (g_per->have_waitpin) {
> + if (g_per->waitpin != idx ||
> + g_per->waitpin_polarity != polarity) {
> + dev_err(gpmc_dev, "error: conflict: waitpin %u with
> polarity %d on device %s.%d\n",
> + g_per->waitpin, g_per->waitpin_polarity,
> + g_per->name, g_per->id);
> + return -EBUSY;
> + }
> + } else {
> + g_per->have_waitpin = true;
> + g_per->waitpin = idx;
> + g_per->waitpin_polarity = polarity;
> + }
> +
> + l |= conf & GPMC_CONFIG1_WAIT_WRITE_MON;
> + l |= conf & GPMC_CONFIG1_WAIT_READ_MON;
> + l &= ~GPMC_CONFIG1_WAIT_PIN_SEL_MASK;
> + l |= GPMC_CONFIG1_WAIT_PIN_SEL(idx);
> +
> + gpmc_cs_write_reg(cs, GPMC_CS_CONFIG1, l);
> +
> + return 0;
> +}
> +
> static inline unsigned gpmc_bit_to_irq(unsigned bitmask)
> {
> return bitmask;
> @@ -1185,6 +1258,55 @@ static __devinit int gpmc_setup_cs_irq(struct
> gpmc_cs_data *cs,
> return n;
> }
>
> +static inline int gpmc_waitpin_is_reserved(unsigned waitpin)
> +{
> + return gpmc_waitpin_map & (0x1 << waitpin);
> +}
> +
> +static inline void gpmc_reserve_waitpin(unsigned waitpin)
> +{
> + gpmc_waitpin_map &= ~(0x1 << waitpin);
> + gpmc_waitpin_map |= (0x1 << waitpin);
> +}
> +
> +static int gpmc_waitpin_request(unsigned waitpin)
> +{
> + if (!(waitpin < GPMC_NR_WAITPIN))
> + return -ENODEV;
> +
> + if (gpmc_waitpin_is_reserved(waitpin))
> + return -EBUSY;
> + else
> + gpmc_re
Re: [PATCH v5 12/14] ARM: OMAP2+: gpmc: cs reconfigure helper
On 06/11/2012 09:27 AM, Afzal Mohammed wrote:
> Helper for reconfiguring CS, peripheral that necessitated
> it was OneNAND.
Why? I think you need to add more about why this was needed.
Jon
> Signed-off-by: Afzal Mohammed
> ---
> arch/arm/mach-omap2/gpmc.c | 32
>
> arch/arm/plat-omap/include/plat/gpmc.h |2 ++
> 2 files changed, 34 insertions(+)
>
> diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> index 281bd23..d493a4c 100644
> --- a/arch/arm/mach-omap2/gpmc.c
> +++ b/arch/arm/mach-omap2/gpmc.c
> @@ -1429,6 +1429,38 @@ struct platform_device *gpmc_create_device(struct
> gpmc_peripheral *p)
> return pdev;
> }
>
> +static int gpmc_match_device(char *name, int id)
> +{
> + int i;
> + struct gpmc_peripheral *g_per = gpmc_peripheral;
> +
> + for (i = 0; i < gpmc_num_peripheral; i++, g_per++)
> + if (!strcmp(g_per->name, name) && g_per->id == id)
> + return i;
> +
> + return -ENOENT;
> +}
> +
> +int gpmc_cs_reconfigure(char *name, int id, struct gpmc_cs_data *c)
> +{
> + int i;
> +
> + i = gpmc_match_device(name, id);
> + if (IS_ERR_VALUE(i)) {
> + dev_err(gpmc_dev, "no device %s.%d to configure\n", name, id);
> + return i;
> + }
> +
> + if (IS_ERR_VALUE(gpmc_setup_cs_config_timing(gpmc_peripheral + i, c))) {
> + dev_err(gpmc_dev, "error: configure device %s.%d\n", name, id);
> + return i;
> + }
> +
> + return gpmc_setup_waitpin(gpmc_peripheral + i);
> +
> +}
> +EXPORT_SYMBOL_GPL(gpmc_cs_reconfigure);
> +
> static __devinit int gpmc_probe(struct platform_device *pdev)
> {
> u32 l;
> diff --git a/arch/arm/plat-omap/include/plat/gpmc.h
> b/arch/arm/plat-omap/include/plat/gpmc.h
> index e1b130c..32017a1 100644
> --- a/arch/arm/plat-omap/include/plat/gpmc.h
> +++ b/arch/arm/plat-omap/include/plat/gpmc.h
> @@ -247,6 +247,8 @@ extern int gpmc_cs_configure(int cs, int cmd, int wval);
> extern int gpmc_nand_read(int cs, int cmd);
> extern int gpmc_nand_write(int cs, int cmd, int wval);
>
> +extern int gpmc_cs_reconfigure(char *name, int id, struct gpmc_cs_data *c);
> +
> int gpmc_enable_hwecc(int cs, int mode, int dev_width, int ecc_size);
> int gpmc_calculate_ecc(int cs, const u_char *dat, u_char *ecc_code);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/3] ARM: OMAP2+: nand: unify init functions
Hi Jon, On Mon, Jun 11, 2012 at 21:13:45, Hunter, Jon wrote: > Which boards have been tested with this change? Beagle board > Reviewed-by: Jon Hunter Thanks Regards Afzal -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: changing DMA slave configuration API
On Mon, 2012-06-11 at 17:33 +0800, Dong Aisheng wrote: > > I think it is a good idea. And I would like to extend it even a > little > > bit. Do we have any users of peripheral to peripheral slave dma? > Yes, IMX sdma does support such kind of transfer. > The driver still does not support it currently. > > > IIRC that is not the case, or does anyone know of existence or > plans > > for such a h/w? > > > i.MX5 and i.MX6. Thanks for confirming, lets keep both. -- ~Vinod -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: changing DMA slave configuration API
On Mon, 2012-06-11 at 09:24 +0100, Russell King - ARM Linux wrote: > On Mon, Jun 11, 2012 at 10:20:49AM +0530, Vinod Koul wrote: > > I think it is a good idea. And I would like to extend it even a little > > bit. Do we have any users of peripheral to peripheral slave dma? > > IIRC that is not the case, or does anyone know of existence or plans > > for such a h/w? > > > > If not, lets junk the src/dst fields and keep burst, length, addr fields > > which point to the peripheral values. > > > > Alternatively if we need both, then we can't have union and Russell's > > idea seems good one :) > > We don't need the union whatever way that goes. Based on comment we need to support both. > > The question over whether we have the src/dst fields is whether we want > to support a different configuration for DMA_DEV_TO_MEM/DMA_MEM_TO_DEV > without having to reconfigure the channel each time its direction is > switched. The biggest issue here is the design of the API. IMO the slave_config should be passed along with respective prepare API for slave and not thru separate slave config. That will remove the unnecessary limitation and allow the same channel to be used for tx for one transfer and rx for subsequent etc. In the .device_prep_slave_sg() we should add the struct slave_config as an argument. Obviously the slave_config will have _one_ pair of members (not both src/dstn fields then). For DMA_DEV_TO_DEV, anyway we need a new API, which can have both src and dstn slave_config Thoughts..? -- ~Vinod -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/3] ARM: OMAP2+: onenand: cleanup for gpmc driver conversion
Hi Jon, On Tue, Jun 12, 2012 at 00:06:30, Hunter, Jon wrote: > I agree with getting rid of the first instance at the beginning of > _set_async_mode, but why get rid of the above one? Are you assuming that > by default it is in async mode? Could be nice to keep it to be explicit. Second one is achieved below > > @@ -191,19 +181,21 @@ static int omap2_onenand_set_sync_mode(struct > > omap_onenand_platform_data *cfg, > > u32 reg; > > bool clk_dep = false; > > > > + /* Ensure sync read and sync write are disabled */ > > + reg = readw(onenand_base + ONENAND_REG_SYS_CFG1); > > + reg &= ~ONENAND_SYS_CFG1_SYNC_READ & ~ONENAND_SYS_CFG1_SYNC_WRITE; > > + writew(reg, onenand_base + ONENAND_REG_SYS_CFG1); > > + > > Should we only set these after we have checked the flags and depending > on which flags are set? Even for sync mode, setting async mode initially is required > > @@ -421,6 +413,8 @@ void __init gpmc_onenand_init(struct > > omap_onenand_platform_data *_onenand_data) > > gpmc_onenand_resource.end = gpmc_onenand_resource.start + > > ONENAND_IO_SIZE - 1; > > > > + omap2_onenand_set_async_mode(gpmc_onenand_data->cs); > > + > > What about putting this in the gpmc_onenand_setup() function before the > call to _set_sync_mode? Seems nice to have these together. Intention of this patch is to setup GPMC async mode before driver starts its job so that reconfiguring GPMC needs to be to be done only once (this helps in avoiding issues when it has to work with gpmc driver). With the existing code, set_async was done as part of set_sync, hence requiring GPMC to be configured twice after driver takes control, with your suggestion too, GPMC would have to be configured twice. Regards Afzal -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/3] ARM: OMAP2+: gpmc: handle additional timings
Hi Jon, On Tue, Jun 12, 2012 at 00:19:35, Hunter, Jon wrote: > What boards have been tested with this change? Beagle board, after applying all 5 series of patches, without all patch series it can't be tested for beagle board as it depended on bootloader, not this API > > + u16 bus_turnaround; > > + u16 cycle2cycle_delay; > > + > > + u16 wait_monitoring; > > + u16 clk_activation; > > + > > /* The following are only on OMAP3430 */ > > u16 wr_access; /* WRACCESSTIME */ > > u16 wr_data_mux_bus;/* WRDATAONADMUXBUS */ > > In general, I agree with this, but if we apply this today, it seems that > we *may* be clearing these fields if they have been configured by the > bootloader and hence this could introduce a regression (potentially). > > So we ever need to test boards that this change impacts or at least > verify that this change would not impact these boards because these > fields have not been configured. Yes, it is the exactly the reason this patch has been splitted out of gpmc driver conversion series. To find out whether enhancing existing API cause any regression and if so, then it can be easily root caused and would not be confused with driver conversion. This was the only change required w.r.t old interface, need to make sure that this won't causes breakage on any of the boards (the boards which were unknowingly depending on bootloader for these settings). I have only beagle board & omap3evm (both would not get affected by this change with existing code) and others can't be validated. Once this is in lo tree or mainline, this change can be validated for different boards. Regards Afzal -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] Input: ads7846: set proper debounce time in driver level
* Zumeng Chen [120611 07:05]:
> If we don't set proper debouce time for ads7846, then there are
> flooded interrupt counters of ads7846 responding to one time
> touch on screen, so the driver couldn't work well.
>
> And since most OMAP3 series boards pass NULL pointer of board_pdata
> to omap_ads7846_init, so it's more proper to set it in driver level
> after having gpio_request done.
>
> This patch has been validated on 3530evm.
>
> Signed-off-by: Zumeng Chen
> Signed-off-by: Syed Mohammed Khasim
> Signed-off-by: Tony Lindgren
Please remove my Signed-off-by, where does that come from?
Tony
> drivers/input/touchscreen/ads7846.c |2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/input/touchscreen/ads7846.c
> b/drivers/input/touchscreen/ads7846.c
> index f02028e..a82a5fb 100644
> --- a/drivers/input/touchscreen/ads7846.c
> +++ b/drivers/input/touchscreen/ads7846.c
> @@ -61,6 +61,7 @@
>
> /* this driver doesn't aim at the peak continuous sample rate */
> #define SAMPLE_BITS (8 /*cmd*/ + 16 /*sample*/ + 2 /* before, after
> */)
> +#define DEBOUNCE_TIME 310 /* About 10 ms */
>
> struct ts_event {
> /*
> @@ -980,6 +981,7 @@ static int __devinit ads7846_setup_pendown(struct
> spi_device *spi, struct ads784
> }
>
> ts->gpio_pendown = pdata->gpio_pendown;
> + gpio_set_debounce(pdata->gpio_pendown, DEBOUNCE_TIME);
>
> } else {
> dev_err(&spi->dev, "no get_pendown_state nor gpio_pendown?\n");
> --
> 1.7.5.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 02/14] ARM: OMAP2+: gpmc: Adapt to HWMOD
Hi Jon,
On Tue, Jun 12, 2012 at 01:26:29, Hunter, Jon wrote:
> > + pdev = omap_device_build(name, -1, oh, pdata,
> > + sizeof(*pdata), NULL, 0, 0);
> > + if (IS_ERR(pdev)) {
> > + WARN(1, "Can't build omap_device for %s:%s.\n",
> > + name, oh->name);
> > + return PTR_ERR(pdev);
> > + }
> > +
> > + gpmc_l3_clk = clk_get(NULL, oh->main_clk);
> > + if (IS_ERR(gpmc_l3_clk)) {
> > + pr_err("Could not get GPMC clock\n");
> > + return PTR_ERR(gpmc_l3_clk);
> > + }
>
> My preference would be to store gpmc_l3_clk in the pdata and pass to
> probe via the pdata. The aim would be to remove the global gpmc_l3_clk
> altogether.
For timing calculation by platform outside of driver, we need clk rate
> Also we should attempt to get the clk before calling omap_device_build
> which is registering the driver.
omap_device_build registers only device, but I agree that clk should
be obtained before it as driver can get probed at that point
Regards,
Afzal
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
