Messages in this thread | | | Date | Fri, 5 Jun 2009 18:25:34 +0200 | Subject | Re: [PATCH 1/7] MFD: ASIC3: add API for EXTCF and SDHWCTRL register manipulation | From | pHilipp Zabel <> |
| |
Hi Samuel,
On Fri, Jun 5, 2009 at 1:46 AM, Samuel Ortiz<sameo@openedhand.com> wrote: > Hi Philipp > > On Thu, Jun 04, 2009 at 08:36:10PM +0200, Philipp Zabel wrote: >> Those registers are needed for PCMCIA and MMC/SDIO operation as >> well as for the DS1WM. >> >> Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com> >> --- >> drivers/mfd/asic3.c | 48 +++++++++++++++++++++++++++++++++++++++++++++ >> include/linux/mfd/asic3.h | 13 +++++++---- >> 2 files changed, 56 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/mfd/asic3.c b/drivers/mfd/asic3.c >> index 9e48545..8e1653a 100644 >> --- a/drivers/mfd/asic3.c >> +++ b/drivers/mfd/asic3.c >> @@ -52,6 +52,54 @@ static inline u32 asic3_read_register(struct asic3 *asic, >> (reg >> asic->bus_shift)); >> } >> >> +void asic3_set_extcf_select(struct device *dev, u32 bits, int value) > 2 remarks here: > - The "value" parameter could be a bool, and could use a better name, e.g. > "set". > - Those 3 routines are basically doing the same thing. Why not having a > generic: > void asic3_set_register(struct device *dev, u32 reg, u32 bits, bool set) ?
Thanks. The reasoning behind this split (besides the fact that the hh.org driver did it this way) was that only asic3_set_extcf_select has to be exported for the pcmcia driver and I'd like to move the register addresses from the public header into asic3.c or into a private header some time.
I think it is better to have one (unexported) function as you suggest and then export a custom function to handle the EXTCF SLEEP_MODE/BUF_EN/PWAIT_EN bits in the future.
Will combine and resend.
> Cheers, > Samuel. > >> +{ >> + struct asic3 *asic = dev->driver_data; >> + unsigned long flags; >> + u32 v; >> + >> + spin_lock_irqsave(&asic->lock, flags); >> + v = asic3_read_register(asic, ASIC3_OFFSET(EXTCF, SELECT)); >> + if (value) >> + v |= bits; >> + else >> + v &= ~bits; >> + asic3_write_register(asic, ASIC3_OFFSET(EXTCF, SELECT), v); >> + spin_unlock_irqrestore(&asic->lock, flags); >> +} >> +EXPORT_SYMBOL(asic3_set_extcf_select); >> + >> +void asic3_set_extcf_reset(struct device *dev, u32 bits, int value) >> +{ >> + struct asic3 *asic = dev->driver_data; >> + unsigned long flags; >> + u32 v; >> + >> + spin_lock_irqsave(&asic->lock, flags); >> + v = asic3_read_register(asic, ASIC3_OFFSET(EXTCF, RESET)); >> + if (value) >> + v |= bits; >> + else >> + v &= ~bits; >> + asic3_write_register(asic, ASIC3_OFFSET(EXTCF, RESET), v); >> + spin_unlock_irqrestore(&asic->lock, flags); >> +} >> + >> +void asic3_set_sdhwctrl(struct asic3 *asic, u32 bits, int value) >> +{ >> + unsigned long flags; >> + u32 v; >> + >> + spin_lock_irqsave(&asic->lock, flags); >> + v = asic3_read_register(asic, ASIC3_OFFSET(SDHWCTRL, SDCONF)); >> + if (value) >> + v |= bits; >> + else >> + v &= ~bits; >> + asic3_write_register(asic, ASIC3_OFFSET(SDHWCTRL, SDCONF), v); >> + spin_unlock_irqrestore(&asic->lock, flags); >> +} >> + >> /* IRQs */ >> #define MAX_ASIC_ISR_LOOPS 20 >> #define ASIC3_GPIO_BASE_INCR \ >> diff --git a/include/linux/mfd/asic3.h b/include/linux/mfd/asic3.h >> index 322cd6d..cd02d20 100644 >> --- a/include/linux/mfd/asic3.h >> +++ b/include/linux/mfd/asic3.h >> @@ -16,6 +16,9 @@ >> >> #include <linux/types.h> >> >> +/* for PCMCIA */ >> +extern void asic3_set_extcf_select(struct device *dev, u32 bits, int value); >> + >> struct asic3_platform_data { >> u16 *gpio_config; >> unsigned int gpio_config_num; >> @@ -227,8 +230,8 @@ struct asic3_platform_data { >> >> >> /* Basic control of the SD ASIC */ >> -#define ASIC3_SDHWCTRL_Base 0x0E00 >> -#define ASIC3_SDHWCTRL_SDConf 0x00 >> +#define ASIC3_SDHWCTRL_BASE 0x0E00 >> +#define ASIC3_SDHWCTRL_SDCONF 0x00 >> >> #define ASIC3_SDHWCTRL_SUSPEND (1 << 0) /* 1=suspend all SD operations */ >> #define ASIC3_SDHWCTRL_CLKSEL (1 << 1) /* 1=SDICK, 0=HCLK */ >> @@ -242,10 +245,10 @@ struct asic3_platform_data { >> /* SD card power supply ctrl 1=enable */ >> #define ASIC3_SDHWCTRL_SDPWR (1 << 6) >> >> -#define ASIC3_EXTCF_Base 0x1100 >> +#define ASIC3_EXTCF_BASE 0x1100 >> >> -#define ASIC3_EXTCF_Select 0x00 >> -#define ASIC3_EXTCF_Reset 0x04 >> +#define ASIC3_EXTCF_SELECT 0x00 >> +#define ASIC3_EXTCF_RESET 0x04 >> >> #define ASIC3_EXTCF_SMOD0 (1 << 0) /* slot number of mode 0 */ >> #define ASIC3_EXTCF_SMOD1 (1 << 1) /* slot number of mode 1 */ >> -- >> 1.6.3.1 >> > > -- > Intel Open Source Technology Centre > http://oss.intel.com/ > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |