[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <74d0deb30906050925va48d17bq9edd4953846ee17d@mail.gmail.com>
Date: Fri, 5 Jun 2009 18:25:34 +0200
From: pHilipp Zabel <philipp.zabel@...il.com>
To: Samuel Ortiz <sameo@...nedhand.com>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/7] MFD: ASIC3: add API for EXTCF and SDHWCTRL register
manipulation
Hi Samuel,
On Fri, Jun 5, 2009 at 1:46 AM, Samuel Ortiz<sameo@...nedhand.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@...il.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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists