* [PATCH 0/3] S5M8751 core driver
@ 2011-06-22 5:53 Sangbeom Kim
2011-06-22 5:53 ` [PATCH 1/3] mfd: Add S5M8751 register definitions Sangbeom Kim
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Sangbeom Kim @ 2011-06-22 5:53 UTC (permalink / raw)
To: sameo; +Cc: linux-kernel
This patch series implements core support for the S5M8751 PMIC.
Since the S5M8751 includes regulator, bettery charger, Audio DAC,
which need be represented via the relevant subsystems
for integration with the rest of the kernel a MFD core device is
provided to manage chip access.
The each part of the driver will be submitted separately
via the relevant subsystems.
[PATCH 1/3] mfd: Add S5M8751 register definitions
[PATCH 2/3] mfd: Add initial S5M8751 support
[PATCH 3/3] mfd: Add I2C control support for S5M8751
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 1/3] mfd: Add S5M8751 register definitions 2011-06-22 5:53 [PATCH 0/3] S5M8751 core driver Sangbeom Kim @ 2011-06-22 5:53 ` Sangbeom Kim 2011-06-22 12:42 ` Mark Brown 2011-06-22 5:53 ` [PATCH 2/3] mfd: Add initial S5M8751 support Sangbeom Kim 2011-06-22 5:53 ` [PATCH 3/3] mfd: Add I2C control support for S5M8751 Sangbeom Kim 2 siblings, 1 reply; 16+ messages in thread From: Sangbeom Kim @ 2011-06-22 5:53 UTC (permalink / raw) To: sameo; +Cc: linux-kernel, Sangbeom Kim This patch add S5M8751 PMIC register definitions. Separated as large code size Signed-off-by: Sangbeom Kim <sbkim73@samsung.com> --- include/linux/mfd/s5m8751.h | 195 +++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 195 insertions(+), 0 deletions(-) create mode 100644 include/linux/mfd/s5m8751.h diff --git a/include/linux/mfd/s5m8751.h b/include/linux/mfd/s5m8751.h new file mode 100644 index 0000000..ca465d0 --- /dev/null +++ b/include/linux/mfd/s5m8751.h @@ -0,0 +1,195 @@ +/* include/linux/mfd/s5m8751.h + * + * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * Register definitions for S5M8751 Advanced PMIC with Audio DAC + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. +*/ + +#ifndef __LINUX_MFD_S5M8751_CORE_H_ +#define __LINUX_MFD_S5M8751_CORE_H_ + +#include <linux/kernel.h> +#include <linux/mutex.h> +#include <linux/workqueue.h> + +#define S5M8751_FRT_REGISTER 0x00 +#define S5M8751_LAT_REGISTER 0x45 +#define S5M8751_SLAVE_ADDRESS 0xD0 + +#define S5M8751_IRQB_EVENT1 0x00 +#define S5M8751_IRQB_EVENT2 0x01 +#define S5M8751_IRQB_MASK1 0x02 +#define S5M8751_IRQB_MASK2 0x03 + +#define S5M8751_ONOFF1 0x04 +#define S5M8751_ONOFF2 0x05 +#define S5M8751_ONOFF3 0x06 + +#define S5M8751_SLEEP_CNTL1 0x07 +#define S5M8751_SLEEP_CNTL2 0x08 +#define S5M8751_UVLO 0x09 + +#define S5M8751_LDO6_VSET 0x0A +#define S5M8751_LDO1_VSET 0x0B +#define S5M8751_LDO2_VSET 0x0C +#define S5M8751_LDO3_VSET 0x0D +#define S5M8751_LDO4_VSET 0x0E +#define S5M8751_LDO5_VSET 0x0F +#define S5M8751_BUCK1_V1_SET 0x10 +#define S5M8751_BUCK1_V2_SET 0x11 +#define S5M8751_BUCK2_V1_SET 0x12 +#define S5M8751_BUCK2_V2_SET 0x13 + +#define S5M8751_WLED_CNTRL 0x14 +#define S5M8751_CHG_IV_SET 0x15 +#define S5M8751_CHG_CNTRL 0x16 + +#define S5M8751_DA_PDB1 0x17 +#define S5M8751_DA_AMIX1 0x18 +#define S5M8751_DA_AMIX2 0x19 +#define S5M8751_DA_ANA 0x1A +#define S5M8751_DA_DWA 0x1B +#define S5M8751_DA_VOLL 0x1C +#define S5M8751_DA_VOLR 0x1D +#define S5M8751_DA_DIG1 0x1E +#define S5M8751_DA_DIG2 0x1F +#define S5M8751_DA_LIM1 0x20 +#define S5M8751_DA_LIM2 0x21 +#define S5M8751_DA_LOF 0x22 +#define S5M8751_DA_ROF 0x23 +#define S5M8751_DA_MUX 0x24 +#define S5M8751_DA_LGAIN 0x25 +#define S5M8751_DA_RGAIN 0x26 +#define S5M8751_IN1_CTRL1 0x27 +#define S5M8751_IN1_CTRL2 0x28 +#define S5M8751_IN1_CTRL3 0x29 +#define S5M8751_SLOT_L2 0x2A +#define S5M8751_SLOT_L1 0x2B +#define S5M8751_SLOT_R2 0x2C +#define S5M8751_SLOT_R1 0x2D +#define S5M8751_TSLOT 0x2E +#define S5M8751_TEST 0x2F + +#define S5M8751_SPK_SLOPE 0x30 +#define S5M8751_SPK_DT 0x31 +#define S5M8751_SPK_S2D 0x32 +#define S5M8751_SPK_CM 0x33 +#define S5M8751_SPK_DUM 0x34 +#define S5M8751_HP_VOL1 0x35 +#define S5M8751_HP_VOL2 0x36 +#define S5M8751_AMP_EN 0x37 +#define S5M8751_AMP_MUTE 0x38 +#define S5M8751_AMP_CTRL 0x39 +#define S5M8751_AMP_VMID 0x3A +#define S5M8751_LINE_CTRL 0x3B + +#define S5M8751_CHG_TEST_WR 0x3C +#define S5M8751_CHG_TRIM 0x3D +#define S5M8751_BUCK_TEST1 0x3E +#define S5M8751_VREF_TEST 0x3F +#define S5M8751_BUCK_TEST2 0x40 +#define S5M8751_LDO_OCPEN 0x42 +#define S5M8751_CHIP_ID 0x43 +#define S5M8751_STATUS 0x44 +#define S5M8751_AUDIO_STATUS 0x45 +#define S5M8751_CHG_TEST_R 0x46 + +#define S5M8751_NUMREGS (S5M8751_CHG_TEST_R+1) +#define S5M8751_MAX_REGISTER 0xFF + +#define S5M8751_IRQ_PWRKEY1B 0 +#define S5M8751_MASK_PWRKEY1B 0x08 +#define S5M8751_IRQ_PWRKEY2B 1 +#define S5M8751_MASK_PWRKEY2B 0x04 +#define S5M8751_IRQ_PWRKEY3 2 +#define S5M8751_MASK_PWRKEY3 0x02 +#define S5M8751_IRQ_VCHG_DETECTION 3 +#define S5M8751_MASK_VCHG_DET 0x10 +#define S5M8751_IRQ_VCHG_REMOVAL 4 +#define S5M8751_MASK_VCHG_REM 0x08 +#define S5M8751_IRQ_CHARGER_TIMEOUT 5 +#define S5M8751_MASK_CHG_T_OUT 0x04 +#define S5M8751_NUM_IRQ 6 + +#define S5M8751_SLEEPB_PIN_ENABLE 0x02 + +struct s5m8751; + +struct s5m8751_irq { + void (*handler) (struct s5m8751 *, int, void *); + void *data; +}; + +struct s5m8751 { + struct device *dev; + + /* device IO */ + struct i2c_client *i2c_client; + + int (*read_dev)(struct s5m8751 *s5m8751, uint8_t reg, uint8_t *val); + int (*write_dev)(struct s5m8751 *s5m8751, uint8_t reg, uint8_t val); + + int (*read_block_dev)(struct s5m8751 *s5m8751, uint8_t reg, int len, + uint8_t *val); + int (*write_block_dev)(struct s5m8751 *s5m8751, uint8_t reg, int len, + uint8_t *val); + u8 *reg_cache; + + /* Interrupt handling */ + struct work_struct irq_work; + struct mutex irq_mutex; /* IRQ table mutex */ + struct s5m8751_irq irq[S5M8751_NUM_IRQ]; + int chip_irq; + + /* Client devices */ +}; + +/* + * Data to be supplied by the platform to initialise the S5m8751. + * + * @init: Function called during driver initialisation. Should be + * used by the platform to configure GPIO functions and similar. + */ +struct s5m8751_platform_data { + int (*init)(struct s5m8751 *s5m8751); +}; + +int s5m8751_device_init(struct s5m8751 *s5m8751, int irq, + struct s5m8751_platform_data *pdata); + +void s5m8751_device_exit(struct s5m8751 *s5m8751); + +/* S5M8751 Device IO */ +int s5m8751_clear_bits(struct s5m8751 *s5m8751, uint8_t reg, uint8_t mask); +int s5m8751_set_bits(struct s5m8751 *s5m8751, uint8_t reg, uint8_t mask); +int s5m8751_reg_read(struct s5m8751 *s5m8751, uint8_t reg, uint8_t *val); +int s5m8751_reg_write(struct s5m8751 *s5m8751, uint8_t reg, uint8_t val); +int s5m8751_block_read(struct s5m8751 *s5m8751, uint8_t reg, int len, + uint8_t *val); +int s5m8751_block_write(struct s5m8751 *s5m8751, uint8_t reg, int len, + uint8_t *val); +/* S5M8751 internal interrupts */ +int s5m8751_register_irq(struct s5m8751 *s5m8751, int irq, + void (*handler) (struct s5m8751 *, int, void *), + void *data); + +int s5m8751_free_irq(struct s5m8751 *s5m8751, int irq); +int s5m8751_mask_irq(struct s5m8751 *s5m8751, int irq); +int s5m8751_unmask_irq(struct s5m8751 *s5m8751, int irq); +int s5m8751_clear_irq(struct s5m8751 *s5m8751); + + +/* S5M8751 sysfs functions */ +int s5m8751_uvlo_get(struct s5m8751 *s5m8751); +int s5m8751_uvlo_set(struct s5m8751 *s5m8751, int voltage); + +int s5m8751_audio_dev_register(struct s5m8751 *s5m8751, + const char *name, + struct platform_device **pdev); + +#endif /* __LINUX_MFD_S5M8751_H_ */ -- 1.7.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] mfd: Add S5M8751 register definitions 2011-06-22 5:53 ` [PATCH 1/3] mfd: Add S5M8751 register definitions Sangbeom Kim @ 2011-06-22 12:42 ` Mark Brown 2011-06-22 23:59 ` Sangbeom Kim 0 siblings, 1 reply; 16+ messages in thread From: Mark Brown @ 2011-06-22 12:42 UTC (permalink / raw) To: Sangbeom Kim; +Cc: sameo, linux-kernel On Wed, Jun 22, 2011 at 02:53:55PM +0900, Sangbeom Kim wrote: > This patch add S5M8751 PMIC register definitions. > Separated as large code size These don't look like register definitions: > +struct s5m8751; > + > +struct s5m8751_irq { > + void (*handler) (struct s5m8751 *, int, void *); > + void *data; > +}; > + > +struct s5m8751 { > + struct device *dev; > + > + /* device IO */ > + struct i2c_client *i2c_client; > + > + int (*read_dev)(struct s5m8751 *s5m8751, uint8_t reg, uint8_t *val); > + int (*write_dev)(struct s5m8751 *s5m8751, uint8_t reg, uint8_t val); > + > + int (*read_block_dev)(struct s5m8751 *s5m8751, uint8_t reg, int len, > + uint8_t *val); > + int (*write_block_dev)(struct s5m8751 *s5m8751, uint8_t reg, int len, > + uint8_t *val); > + u8 *reg_cache; > + > + /* Interrupt handling */ > + struct work_struct irq_work; > + struct mutex irq_mutex; /* IRQ table mutex */ > + struct s5m8751_irq irq[S5M8751_NUM_IRQ]; > + int chip_irq; > + > + /* Client devices */ > +}; > +/* S5M8751 Device IO */ > +int s5m8751_clear_bits(struct s5m8751 *s5m8751, uint8_t reg, uint8_t mask); > +int s5m8751_set_bits(struct s5m8751 *s5m8751, uint8_t reg, uint8_t mask); > +int s5m8751_reg_read(struct s5m8751 *s5m8751, uint8_t reg, uint8_t *val); > +int s5m8751_reg_write(struct s5m8751 *s5m8751, uint8_t reg, uint8_t val); > +int s5m8751_block_read(struct s5m8751 *s5m8751, uint8_t reg, int len, > + uint8_t *val); > +int s5m8751_block_write(struct s5m8751 *s5m8751, uint8_t reg, int len, > + uint8_t *val); I posted a series earlier this week which adds a generic API for handling register maps on I2C/SPI devices - I'll post another version of it shortly, it'd be nice if you could loook to see if this is usable for you. > +/* S5M8751 internal interrupts */ > +int s5m8751_register_irq(struct s5m8751 *s5m8751, int irq, > + void (*handler) (struct s5m8751 *, int, void *), > + void *data); > + > +int s5m8751_free_irq(struct s5m8751 *s5m8751, int irq); > +int s5m8751_mask_irq(struct s5m8751 *s5m8751, int irq); > +int s5m8751_unmask_irq(struct s5m8751 *s5m8751, int irq); > +int s5m8751_clear_irq(struct s5m8751 *s5m8751); You should use genirq for this, it's supported this for quite some time now. There's quite a few MFDs doing this which you can used for examples. > +/* S5M8751 sysfs functions */ > +int s5m8751_uvlo_get(struct s5m8751 *s5m8751); > +int s5m8751_uvlo_set(struct s5m8751 *s5m8751, int voltage); > + > +int s5m8751_audio_dev_register(struct s5m8751 *s5m8751, > + const char *name, > + struct platform_device **pdev); These shouldn't need to be in the header. ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 1/3] mfd: Add S5M8751 register definitions 2011-06-22 12:42 ` Mark Brown @ 2011-06-22 23:59 ` Sangbeom Kim 0 siblings, 0 replies; 16+ messages in thread From: Sangbeom Kim @ 2011-06-22 23:59 UTC (permalink / raw) To: 'Mark Brown'; +Cc: sameo, linux-kernel Hi Mark, On Wed, Jun 22, 2011 at 09:42PM +0900, Mark Brown wrote: > These don't look like register definitions: OK, I will move it, > > I posted a series earlier this week which adds a generic API for > handling register maps on I2C/SPI devices - I'll post another version of > it shortly, it'd be nice if you could loook to see if this is usable for > you. > Thanks for your comments. After check the your patch, I will remake S5M8751 patch. > > +/* S5M8751 internal interrupts */ > > +int s5m8751_register_irq(struct s5m8751 *s5m8751, int irq, > > + void (*handler) (struct s5m8751 *, int, void *), > > + void *data); > > + > > +int s5m8751_free_irq(struct s5m8751 *s5m8751, int irq); > > +int s5m8751_mask_irq(struct s5m8751 *s5m8751, int irq); > > +int s5m8751_unmask_irq(struct s5m8751 *s5m8751, int irq); > > +int s5m8751_clear_irq(struct s5m8751 *s5m8751); > > You should use genirq for this, it's supported this for quite some time > now. There's quite a few MFDs doing this which you can used for > examples. OK I will check it. > > +/* S5M8751 sysfs functions */ > > +int s5m8751_uvlo_get(struct s5m8751 *s5m8751); > > +int s5m8751_uvlo_set(struct s5m8751 *s5m8751, int voltage); > > + > > +int s5m8751_audio_dev_register(struct s5m8751 *s5m8751, > > + const char *name, > > + struct platform_device **pdev); > > These shouldn't need to be in the header. OK, I will modify it. Thanks and regards, ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] mfd: Add initial S5M8751 support 2011-06-22 5:53 [PATCH 0/3] S5M8751 core driver Sangbeom Kim 2011-06-22 5:53 ` [PATCH 1/3] mfd: Add S5M8751 register definitions Sangbeom Kim @ 2011-06-22 5:53 ` Sangbeom Kim 2011-06-22 12:48 ` Mark Brown 2011-07-04 14:07 ` Samuel Ortiz 2011-06-22 5:53 ` [PATCH 3/3] mfd: Add I2C control support for S5M8751 Sangbeom Kim 2 siblings, 2 replies; 16+ messages in thread From: Sangbeom Kim @ 2011-06-22 5:53 UTC (permalink / raw) To: sameo; +Cc: linux-kernel, Sangbeom Kim The WM8994 is a advanced PMIC with AUdio DAC Since it includes regulators, Battery charger, audio dac, it is represented as a multi-function device, though the functionality will be provided by the each driver. Signed-off-by: Sangbeom Kim <sbkim73@samsung.com> --- drivers/mfd/Kconfig | 3 + drivers/mfd/Makefile | 2 + drivers/mfd/s5m8751-core.c | 358 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 363 insertions(+), 0 deletions(-) create mode 100644 drivers/mfd/s5m8751-core.c diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 0f09c05..85c91ac 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -385,6 +385,9 @@ config MFD_WM831X_SPI for accessing the device, additional drivers must be enabled in order to use the functionality of the device. +config MFD_S5M8751 + tristate + config MFD_WM8350 bool depends on GENERIC_HARDIRQS diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index efe3cc3..27f99bb 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -94,3 +94,5 @@ obj-$(CONFIG_MFD_OMAP_USB_HOST) += omap-usb-host.o obj-$(CONFIG_MFD_PM8921_CORE) += pm8921-core.o obj-$(CONFIG_MFD_PM8XXX_IRQ) += pm8xxx-irq.o obj-$(CONFIG_MFD_TPS65910) += tps65910.o tps65910-irq.o +s5m8751-objs += s5m8751-core.o +obj-$(CONFIG_MFD_S5M8751) += s5m8751.o diff --git a/drivers/mfd/s5m8751-core.c b/drivers/mfd/s5m8751-core.c new file mode 100644 index 0000000..3f15d08 --- /dev/null +++ b/drivers/mfd/s5m8751-core.c @@ -0,0 +1,358 @@ +/* linux/drivers/mfd/s5m8751-core.c + * + * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * S5M8751 core driver + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. +*/ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/init.h> +#include <linux/device.h> +#include <linux/interrupt.h> +#include <linux/workqueue.h> +#include <linux/platform_device.h> + +#include <linux/mfd/s5m8751.h> + +#define SLEEPB_ENABLE 1 +#define SLEEPB_DISABLE 0 + +static DEFINE_MUTEX(io_mutex); + +int s5m8751_clear_bits(struct s5m8751 *s5m8751, uint8_t reg, uint8_t mask) +{ + uint8_t reg_val; + int ret = 0; + + ret = s5m8751_reg_read(s5m8751, reg, ®_val); + if (ret) + return ret; + + reg_val &= ~mask; + ret = s5m8751_reg_write(s5m8751, reg, reg_val); + + return ret; +} +EXPORT_SYMBOL_GPL(s5m8751_clear_bits); + +int s5m8751_set_bits(struct s5m8751 *s5m8751, uint8_t reg, uint8_t mask) +{ + uint8_t reg_val; + int ret = 0; + + ret = s5m8751_reg_read(s5m8751, reg, ®_val); + if (ret) + return ret; + + reg_val |= mask; + ret = s5m8751_reg_write(s5m8751, reg, reg_val); + + return ret; +} +EXPORT_SYMBOL_GPL(s5m8751_set_bits); + +int s5m8751_reg_read(struct s5m8751 *s5m8751, uint8_t reg, uint8_t *val) +{ + int ret = 0; + + mutex_lock(&io_mutex); + ret = s5m8751->read_dev(s5m8751, reg, val); + if (ret < 0) + dev_err(s5m8751->dev, "failed reading from 0x%02x\n", reg); + + mutex_unlock(&io_mutex); + return ret; +} +EXPORT_SYMBOL_GPL(s5m8751_reg_read); + +int s5m8751_reg_write(struct s5m8751 *s5m8751, uint8_t reg, uint8_t val) +{ + int ret = 0; + + mutex_lock(&io_mutex); + ret = s5m8751->write_dev(s5m8751, reg, val); + if (ret < 0) + dev_err(s5m8751->dev, "failed writing 0x%02x\n", reg); + + mutex_unlock(&io_mutex); + return ret; +} +EXPORT_SYMBOL_GPL(s5m8751_reg_write); + +int s5m8751_block_read(struct s5m8751 *s5m8751, uint8_t reg, int len, + uint8_t *val) +{ + int ret = 0; + + mutex_lock(&io_mutex); + ret = s5m8751->read_block_dev(s5m8751, reg, len, val); + if (ret < 0) + dev_err(s5m8751->dev, "failed reading from 0x%02x\n", reg); + + mutex_unlock(&io_mutex); + return ret; +} +EXPORT_SYMBOL_GPL(s5m8751_block_read); + +int s5m8751_block_write(struct s5m8751 *s5m8751, uint8_t reg, int len, + uint8_t *val) +{ + int ret = 0; + + mutex_lock(&io_mutex); + ret = s5m8751->write_block_dev(s5m8751, reg, len, val); + if (ret < 0) + dev_err(s5m8751->dev, "failed writings to 0x%02x\n", reg); + + mutex_unlock(&io_mutex); + return ret; +} +EXPORT_SYMBOL_GPL(s5m8751_block_write); + +static void s5m8751_irq_call_handler(struct s5m8751 *s5m8751, int irq) +{ + mutex_lock(&s5m8751->irq_mutex); + + if (s5m8751->irq[irq].handler) + s5m8751->irq[irq].handler(s5m8751, irq, s5m8751->irq[irq].data); + else { + dev_err(s5m8751->dev, "irq %d nobody cared. now masked.\n", + irq); + s5m8751_mask_irq(s5m8751, irq); + } + + mutex_unlock(&s5m8751->irq_mutex); +} + +/* + * s5m8751_irq_worker actually handles the interrupts. + * All interrupts must be clear after reading the event registers. + */ +static void s5m8751_irq_worker(struct work_struct *work) +{ + struct s5m8751 *s5m8751 = container_of(work, struct s5m8751, irq_work); + uint8_t event1, event2, mask1, mask2; + + s5m8751_reg_read(s5m8751, S5M8751_IRQB_EVENT1, &event1); + s5m8751_reg_read(s5m8751, S5M8751_IRQB_EVENT2, &event2); + s5m8751_reg_read(s5m8751, S5M8751_IRQB_MASK1, &mask1); + s5m8751_reg_read(s5m8751, S5M8751_IRQB_MASK2, &mask2); + + event1 &= ~mask1; + event2 &= ~mask2; + + if (event1 & S5M8751_MASK_PWRKEY1B) + s5m8751_irq_call_handler(s5m8751, S5M8751_IRQ_PWRKEY1B); + + if (event1 & S5M8751_MASK_PWRKEY2B) + s5m8751_irq_call_handler(s5m8751, S5M8751_IRQ_PWRKEY2B); + + if (event1 & S5M8751_MASK_PWRKEY3) + s5m8751_irq_call_handler(s5m8751, S5M8751_IRQ_PWRKEY3); + + if (event2 & S5M8751_MASK_VCHG_DET) + s5m8751_irq_call_handler(s5m8751, S5M8751_IRQ_VCHG_DETECTION); + + if (event2 & S5M8751_MASK_VCHG_REM) + s5m8751_irq_call_handler(s5m8751, + S5M8751_IRQ_VCHG_REMOVAL); + + if (event2 & S5M8751_MASK_CHG_T_OUT) + s5m8751_irq_call_handler(s5m8751, S5M8751_IRQ_CHARGER_TIMEOUT); + + enable_irq(s5m8751->chip_irq); + s5m8751_clear_irq(s5m8751); +} + +static irqreturn_t s5m8751_irq(int irq, void *data) +{ + struct s5m8751 *s5m8751 = data; + + disable_irq_nosync(irq); + schedule_work(&s5m8751->irq_work); + + return IRQ_HANDLED; +} + +int s5m8751_register_irq(struct s5m8751 *s5m8751, int irq, + void (*handler) (struct s5m8751 *, int, void *), + void *data) + +{ + if (irq < 0 || irq > S5M8751_NUM_IRQ || !handler) + return -EINVAL; + + if (s5m8751->irq[irq].handler) + return -EBUSY; + + mutex_lock(&s5m8751->irq_mutex); + s5m8751->irq[irq].handler = handler; + s5m8751->irq[irq].data = data; + mutex_unlock(&s5m8751->irq_mutex); + + return 0; +} +EXPORT_SYMBOL_GPL(s5m8751_register_irq); + +int s5m8751_free_irq(struct s5m8751 *s5m8751, int irq) +{ + if (irq < 0 || irq > S5M8751_NUM_IRQ) + return -EINVAL; + + mutex_lock(&s5m8751->irq_mutex); + s5m8751->irq[irq].handler = NULL; + mutex_unlock(&s5m8751->irq_mutex); + return 0; +} +EXPORT_SYMBOL_GPL(s5m8751_free_irq); + +int s5m8751_mask_irq(struct s5m8751 *s5m8751, int irq) +{ + switch (irq) { + case S5M8751_IRQ_PWRKEY1B: + return s5m8751_set_bits(s5m8751, S5M8751_IRQB_MASK1, + S5M8751_MASK_PWRKEY1B); + case S5M8751_IRQ_PWRKEY2B: + return s5m8751_set_bits(s5m8751, S5M8751_IRQB_MASK1, + S5M8751_MASK_PWRKEY2B); + case S5M8751_IRQ_PWRKEY3: + return s5m8751_set_bits(s5m8751, S5M8751_IRQB_MASK1, + S5M8751_MASK_PWRKEY3); + case S5M8751_IRQ_VCHG_DETECTION: + return s5m8751_set_bits(s5m8751, S5M8751_IRQB_MASK2, + S5M8751_MASK_VCHG_DET); + case S5M8751_IRQ_VCHG_REMOVAL: + return s5m8751_set_bits(s5m8751, S5M8751_IRQB_MASK2, + S5M8751_MASK_VCHG_REM); + case S5M8751_IRQ_CHARGER_TIMEOUT: + return s5m8751_set_bits(s5m8751, S5M8751_IRQB_MASK2, + S5M8751_MASK_CHG_T_OUT); + default: + dev_warn(s5m8751->dev, "Attempting to unmask unknown IRQ %d\n", + irq); + return -EINVAL; + } + return 0; +} +EXPORT_SYMBOL_GPL(s5m8751_mask_irq); + +int s5m8751_unmask_irq(struct s5m8751 *s5m8751, int irq) +{ + switch (irq) { + case S5M8751_IRQ_PWRKEY1B: + return s5m8751_clear_bits(s5m8751, S5M8751_IRQB_MASK1, + S5M8751_MASK_PWRKEY1B); + case S5M8751_IRQ_PWRKEY2B: + return s5m8751_clear_bits(s5m8751, S5M8751_IRQB_MASK1, + S5M8751_MASK_PWRKEY2B); + case S5M8751_IRQ_PWRKEY3: + return s5m8751_clear_bits(s5m8751, S5M8751_IRQB_MASK1, + S5M8751_MASK_PWRKEY3); + case S5M8751_IRQ_VCHG_DETECTION: + return s5m8751_clear_bits(s5m8751, S5M8751_IRQB_MASK2, + S5M8751_MASK_VCHG_DET); + case S5M8751_IRQ_VCHG_REMOVAL: + return s5m8751_clear_bits(s5m8751, S5M8751_IRQB_MASK2, + S5M8751_MASK_VCHG_REM); + case S5M8751_IRQ_CHARGER_TIMEOUT: + return s5m8751_clear_bits(s5m8751, S5M8751_IRQB_MASK2, + S5M8751_MASK_CHG_T_OUT); + default: + dev_warn(s5m8751->dev, "Attempting to unmask unknown IRQ %d\n", + irq); + return -EINVAL; + } + return 0; +} +EXPORT_SYMBOL_GPL(s5m8751_unmask_irq); + +int s5m8751_clear_irq(struct s5m8751 *s5m8751) +{ + uint8_t event = 0x00; + + s5m8751_reg_write(s5m8751, S5M8751_IRQB_EVENT1, event); + s5m8751_reg_write(s5m8751, S5M8751_IRQB_EVENT2, event); + + return 0; +} +EXPORT_SYMBOL_GPL(s5m8751_clear_irq); + +static int s5m8751_sleepb_set(struct s5m8751 *s5m8751, int enable) +{ + if (enable) + s5m8751_set_bits(s5m8751, S5M8751_ONOFF1, + S5M8751_SLEEPB_PIN_ENABLE); + else + s5m8751_clear_bits(s5m8751, S5M8751_ONOFF1, + S5M8751_SLEEPB_PIN_ENABLE); + return 0; +} + +int s5m8751_device_init(struct s5m8751 *s5m8751, int irq, + struct s5m8751_platform_data *pdata) +{ + int ret = -EINVAL; + u8 chip_id; + + if (pdata->init) { + ret = pdata->init(s5m8751); + if (ret != 0) { + dev_err(s5m8751->dev, + "Platform init() failed: %d\n", ret); + goto err; + } + } + + s5m8751_reg_read(s5m8751, S5M8751_CHIP_ID, &chip_id); + if (!chip_id) + dev_info(s5m8751->dev, "Found S5M8751 device\n"); + else { + dev_info(s5m8751->dev, "Didn't Find S5M8751 device\n"); + ret = -EINVAL; + goto err; + } + + s5m8751_clear_irq(s5m8751); + s5m8751_sleepb_set(s5m8751, SLEEPB_ENABLE); + + mutex_init(&s5m8751->irq_mutex); + INIT_WORK(&s5m8751->irq_work, s5m8751_irq_worker); + if (irq) { + ret = request_irq(irq, s5m8751_irq, 0, + "s5m8751", s5m8751); + if (ret != 0) { + dev_err(s5m8751->dev, "Failed to request IRQ: %d\n", + ret); + goto err; + } + } else { + dev_err(s5m8751->dev, "No IRQ configured\n"); + goto err; + } + s5m8751->chip_irq = irq; + + return 0; + +err: + return ret; +} +EXPORT_SYMBOL_GPL(s5m8751_device_init); + +void s5m8751_device_exit(struct s5m8751 *s5m8751) +{ + dev_info(s5m8751->dev, "Unloaded S5M8751 device\n"); + + free_irq(s5m8751->chip_irq, s5m8751); + flush_work(&s5m8751->irq_work); +} +EXPORT_SYMBOL_GPL(s5m8751_device_exit); + +MODULE_DESCRIPTION("Core driver for Samsung S5M8751"); +MODULE_AUTHOR("Sangbeom Kim <sbkim73@samsung.com>"); +MODULE_LICENSE("GPL"); -- 1.7.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] mfd: Add initial S5M8751 support 2011-06-22 5:53 ` [PATCH 2/3] mfd: Add initial S5M8751 support Sangbeom Kim @ 2011-06-22 12:48 ` Mark Brown 2011-06-23 1:14 ` Sangbeom Kim 2011-07-04 14:07 ` Samuel Ortiz 1 sibling, 1 reply; 16+ messages in thread From: Mark Brown @ 2011-06-22 12:48 UTC (permalink / raw) To: Sangbeom Kim; +Cc: sameo, linux-kernel On Wed, Jun 22, 2011 at 02:53:56PM +0900, Sangbeom Kim wrote: > The WM8994 is a advanced PMIC with AUdio DAC > Since it includes regulators, Battery charger, audio dac, > it is represented as a multi-function device, > though the functionality will be provided by the each driver. I think you may have cut'n'pasted bits of this changelog from somewhere else without doing all the updates you meant to :) > + if (event1 & S5M8751_MASK_PWRKEY1B) > + s5m8751_irq_call_handler(s5m8751, S5M8751_IRQ_PWRKEY1B); > + > + if (event1 & S5M8751_MASK_PWRKEY2B) > + s5m8751_irq_call_handler(s5m8751, S5M8751_IRQ_PWRKEY2B); > + > + if (event1 & S5M8751_MASK_PWRKEY3) > + s5m8751_irq_call_handler(s5m8751, S5M8751_IRQ_PWRKEY3); This looks like the IRQ handler code doesn't need to understand the specific events, it can just loop over the registers and fire off interrupts based on the bit numbers. As I said for the first patch this should all be using genirq. > +static irqreturn_t s5m8751_irq(int irq, void *data) > +{ > + struct s5m8751 *s5m8751 = data; > + > + disable_irq_nosync(irq); > + schedule_work(&s5m8751->irq_work); > + > + return IRQ_HANDLED; > +} Use a threaded IRQ handler. > +int s5m8751_device_init(struct s5m8751 *s5m8751, int irq, > + struct s5m8751_platform_data *pdata) > +{ > + int ret = -EINVAL; > + u8 chip_id; > + > + if (pdata->init) { > + ret = pdata->init(s5m8751); > + if (ret != 0) { > + dev_err(s5m8751->dev, > + "Platform init() failed: %d\n", ret); > + goto err; > + } > + } > + > + s5m8751_reg_read(s5m8751, S5M8751_CHIP_ID, &chip_id); > + if (!chip_id) > + dev_info(s5m8751->dev, "Found S5M8751 device\n"); > + else { > + dev_info(s5m8751->dev, "Didn't Find S5M8751 device\n"); > + ret = -EINVAL; > + goto err; > + } I'd really expect the init() callback to run after we've checked that we can talk to the device. ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 2/3] mfd: Add initial S5M8751 support 2011-06-22 12:48 ` Mark Brown @ 2011-06-23 1:14 ` Sangbeom Kim 0 siblings, 0 replies; 16+ messages in thread From: Sangbeom Kim @ 2011-06-23 1:14 UTC (permalink / raw) To: 'Mark Brown'; +Cc: sameo, linux-kernel On Wed, Jun 22, 2011 at 09:48 PM +0900, Mark Brown wrote: > > The WM8994 is a advanced PMIC with AUdio DAC > > Since it includes regulators, Battery charger, audio dac, > > it is represented as a multi-function device, > > though the functionality will be provided by the each driver. > > I think you may have cut'n'pasted bits of this changelog from somewhere > else without doing all the updates you meant to :) Sorry, These days I work with 2 board based on S5M8751 & WM8994. I little confused 8751 with 8994. I will modify it in new patch. > > + if (event1 & S5M8751_MASK_PWRKEY1B) > > + s5m8751_irq_call_handler(s5m8751, S5M8751_IRQ_PWRKEY1B); > > + > > + if (event1 & S5M8751_MASK_PWRKEY2B) > > + s5m8751_irq_call_handler(s5m8751, S5M8751_IRQ_PWRKEY2B); > > + > > + if (event1 & S5M8751_MASK_PWRKEY3) > > + s5m8751_irq_call_handler(s5m8751, S5M8751_IRQ_PWRKEY3); > > This looks like the IRQ handler code doesn't need to understand the > specific events, it can just loop over the registers and fire off > interrupts based on the bit numbers. > > As I said for the first patch this should all be using genirq. OK I will modify it. > > > +static irqreturn_t s5m8751_irq(int irq, void *data) > > +{ > > + struct s5m8751 *s5m8751 = data; > > + > > + disable_irq_nosync(irq); > > + schedule_work(&s5m8751->irq_work); > > + > > + return IRQ_HANDLED; > > +} > > Use a threaded IRQ handler. OK, > > > +int s5m8751_device_init(struct s5m8751 *s5m8751, int irq, > > + struct s5m8751_platform_data *pdata) > > +{ > > + int ret = -EINVAL; > > + u8 chip_id; > > + > > + if (pdata->init) { > > + ret = pdata->init(s5m8751); > > + if (ret != 0) { > > + dev_err(s5m8751->dev, > > + "Platform init() failed: %d\n", ret); > > + goto err; > > + } > > + } > > + > > + s5m8751_reg_read(s5m8751, S5M8751_CHIP_ID, &chip_id); > > + if (!chip_id) > > + dev_info(s5m8751->dev, "Found S5M8751 device\n"); > > + else { > > + dev_info(s5m8751->dev, "Didn't Find S5M8751 device\n"); > > + ret = -EINVAL; > > + goto err; > > + } > > I'd really expect the init() callback to run after we've checked that > we can talk to the device. OK, I will check it. Thanks and regards, SB Kim ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] mfd: Add initial S5M8751 support 2011-06-22 5:53 ` [PATCH 2/3] mfd: Add initial S5M8751 support Sangbeom Kim 2011-06-22 12:48 ` Mark Brown @ 2011-07-04 14:07 ` Samuel Ortiz 2011-07-04 23:49 ` Sangbeom Kim 1 sibling, 1 reply; 16+ messages in thread From: Samuel Ortiz @ 2011-07-04 14:07 UTC (permalink / raw) To: Sangbeom Kim; +Cc: linux-kernel Hi, On Wed, Jun 22, 2011 at 02:53:56PM +0900, Sangbeom Kim wrote: > +#define SLEEPB_ENABLE 1 > +#define SLEEPB_DISABLE 0 > + > +static DEFINE_MUTEX(io_mutex); I would prefer to see your IO mutex defined from your s5m8751 structure. > +int s5m8751_clear_bits(struct s5m8751 *s5m8751, uint8_t reg, uint8_t mask) > +{ > + uint8_t reg_val; > + int ret = 0; > + > + ret = s5m8751_reg_read(s5m8751, reg, ®_val); > + if (ret) > + return ret; > + > + reg_val &= ~mask; > + ret = s5m8751_reg_write(s5m8751, reg, reg_val); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(s5m8751_clear_bits); > + > +int s5m8751_set_bits(struct s5m8751 *s5m8751, uint8_t reg, uint8_t mask) > +{ > + uint8_t reg_val; > + int ret = 0; > + > + ret = s5m8751_reg_read(s5m8751, reg, ®_val); > + if (ret) > + return ret; > + > + reg_val |= mask; > + ret = s5m8751_reg_write(s5m8751, reg, reg_val); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(s5m8751_set_bits); Your locking for both of those routines is also racy. There's nothing preventing a writre to happen between your read and your write. They need to happen atomically, and for that you need to take the lonk in the clear/set bits routine. Cheers, Samuel. -- Intel Open Source Technology Centre http://oss.intel.com/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 2/3] mfd: Add initial S5M8751 support 2011-07-04 14:07 ` Samuel Ortiz @ 2011-07-04 23:49 ` Sangbeom Kim 0 siblings, 0 replies; 16+ messages in thread From: Sangbeom Kim @ 2011-07-04 23:49 UTC (permalink / raw) To: 'Samuel Ortiz'; +Cc: linux-kernel Hi, Thanks for your kindly comment. On Monday, July 04, 2011 11:08 PM +0900, Samuel Ortiz wrote: > On Wed, Jun 22, 2011 at 02:53:56PM +0900, Sangbeom Kim wrote: > > +#define SLEEPB_ENABLE 1 > > +#define SLEEPB_DISABLE 0 > > + > > +static DEFINE_MUTEX(io_mutex); > I would prefer to see your IO mutex defined from your s5m8751 structure. Ok, I will make IO mutex in s5m8751 structure in the next version. > > > +int s5m8751_clear_bits(struct s5m8751 *s5m8751, uint8_t reg, uint8_t > mask) > > +{ > > + uint8_t reg_val; > > + int ret = 0; > > + > > + ret = s5m8751_reg_read(s5m8751, reg, ®_val); > > + if (ret) > > + return ret; > > + > > + reg_val &= ~mask; > > + ret = s5m8751_reg_write(s5m8751, reg, reg_val); > > + > > + return ret; > > +} > > +EXPORT_SYMBOL_GPL(s5m8751_clear_bits); > > + > > +int s5m8751_set_bits(struct s5m8751 *s5m8751, uint8_t reg, uint8_t mask) > > +{ > > + uint8_t reg_val; > > + int ret = 0; > > + > > + ret = s5m8751_reg_read(s5m8751, reg, ®_val); > > + if (ret) > > + return ret; > > + > > + reg_val |= mask; > > + ret = s5m8751_reg_write(s5m8751, reg, reg_val); > > + > > + return ret; > > +} > > +EXPORT_SYMBOL_GPL(s5m8751_set_bits); > Your locking for both of those routines is also racy. There's nothing > preventing a writre to happen between your read and your write. They need > to > happen atomically, and for that you need to take the lonk in the clear/set > bits routine. Ok, I will add mutex_lock between read and write in the next version. Sooner or later, I will submit new patches. Thanks, SB Kim ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] mfd: Add I2C control support for S5M8751 2011-06-22 5:53 [PATCH 0/3] S5M8751 core driver Sangbeom Kim 2011-06-22 5:53 ` [PATCH 1/3] mfd: Add S5M8751 register definitions Sangbeom Kim 2011-06-22 5:53 ` [PATCH 2/3] mfd: Add initial S5M8751 support Sangbeom Kim @ 2011-06-22 5:53 ` Sangbeom Kim 2011-06-22 8:56 ` Maxin B John 2011-06-22 12:50 ` Mark Brown 2 siblings, 2 replies; 16+ messages in thread From: Sangbeom Kim @ 2011-06-22 5:53 UTC (permalink / raw) To: sameo; +Cc: linux-kernel, Sangbeom Kim Implement the I2C control interface for the S5M8751. Signed-off-by: Sangbeom Kim <sbkim73@samsung.com> --- drivers/mfd/Kconfig | 9 +++ drivers/mfd/Makefile | 2 + drivers/mfd/s5m8751-i2c.c | 141 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 152 insertions(+), 0 deletions(-) create mode 100644 drivers/mfd/s5m8751-i2c.c diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 85c91ac..4a8468c 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -388,6 +388,15 @@ config MFD_WM831X_SPI config MFD_S5M8751 tristate +config MFD_S5M8751_I2C + tristate "Support Samsung S5M8751 PMIC/Audio DAC with I2C" + select MFD_S5M8751 + select MFD_CORE + depends on I2C + help + S5M8751 is an advanced power management with AUDIO DAC. This option + enables chip support for the S5M8751 with I2C as the control interface. + config MFD_WM8350 bool depends on GENERIC_HARDIRQS diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index 27f99bb..b94476e 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -96,3 +96,5 @@ obj-$(CONFIG_MFD_PM8XXX_IRQ) += pm8xxx-irq.o obj-$(CONFIG_MFD_TPS65910) += tps65910.o tps65910-irq.o s5m8751-objs += s5m8751-core.o obj-$(CONFIG_MFD_S5M8751) += s5m8751.o +obj-$(CONFIG_MFD_S5M8751_I2C) += s5m8751-i2c.o + diff --git a/drivers/mfd/s5m8751-i2c.c b/drivers/mfd/s5m8751-i2c.c new file mode 100644 index 0000000..4ca680b --- /dev/null +++ b/drivers/mfd/s5m8751-i2c.c @@ -0,0 +1,141 @@ +/* linux/drivers/mfd/s5m8751-i2s.c + * + * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * I2C driver for S5M8751 + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. +*/ + +#include <linux/module.h> +#include <linux/moduleparam.h> +#include <linux/init.h> +#include <linux/interrupt.h> +#include <linux/platform_device.h> +#include <linux/i2c.h> +#include <linux/mfd/s5m8751.h> +#include <linux/slab.h> + +static int s5m8751_i2c_read_device(struct s5m8751 *s5m8751, uint8_t reg, + uint8_t *val) +{ + int ret; + ret = i2c_smbus_read_byte_data(s5m8751->i2c_client, reg); + if (ret < 0) { + dev_err(s5m8751->dev, "failed reading at 0x%02x\n", reg); + return ret; + } + *val = (uint8_t)ret; + return 0; +} + +static int s5m8751_i2c_write_device(struct s5m8751 *s5m8751, uint8_t reg, + uint8_t val) +{ + int ret; + ret = i2c_smbus_write_byte_data(s5m8751->i2c_client, reg, val); + if (ret < 0) { + dev_err(s5m8751->dev, "failed writing 0x%02x to 0x%02x\n", + val, reg); + return ret; + } + return 0; +} + +static int s5m8751_i2c_write_block_device(struct s5m8751 *s5m8751, uint8_t reg, + int len, uint8_t *val) +{ + int ret; + ret = i2c_smbus_write_i2c_block_data(s5m8751->i2c_client, reg, len, + val); + if (ret < 0) { + dev_err(s5m8751->dev, "failed writings to 0x%02x\n", reg); + return ret; + } + return 0; +} + +static int s5m8751_i2c_read_block_device(struct s5m8751 *s5m8751, uint8_t reg, + int len, uint8_t *val) +{ + int ret; + ret = i2c_smbus_read_i2c_block_data(s5m8751->i2c_client, reg, len, val); + if (ret < 0) { + dev_err(s5m8751->dev, "failed reading from 0x%02x\n", reg); + return ret; + } + return 0; +} + +static int s5m8751_probe(struct i2c_client *i2c, + const struct i2c_device_id *id) +{ + struct s5m8751 *s5m8751; + int ret = 0; + + s5m8751 = kzalloc(sizeof(struct s5m8751), GFP_KERNEL); + if (s5m8751 == NULL) + goto err; + + i2c_set_clientdata(i2c, s5m8751); + + s5m8751->dev = &i2c->dev; + s5m8751->i2c_client = i2c; + s5m8751->read_dev = s5m8751_i2c_read_device; + s5m8751->write_dev = s5m8751_i2c_write_device; + s5m8751->read_block_dev = s5m8751_i2c_read_block_device; + s5m8751->write_block_dev = s5m8751_i2c_write_block_device; + + ret = s5m8751_device_init(s5m8751, i2c->irq, i2c->dev.platform_data); + if (ret < 0) + goto err; + + return 0; + +err: + return ret; +} + +static int s5m8751_remove(struct i2c_client *i2c) +{ + struct s5m8751 *s5m8751 = i2c_get_clientdata(i2c); + + s5m8751_device_exit(s5m8751); + + return 0; +} + +static const struct i2c_device_id s5m8751_i2c_id[] = { + { "s5m8751", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, s5m8751_i2c_id); + +static struct i2c_driver s5m8751_driver = { + .driver = { + .name = "s5m8751", + .owner = THIS_MODULE, + }, + .probe = s5m8751_probe, + .remove = s5m8751_remove, + .id_table = s5m8751_i2c_id, +}; + +static int __init s5m8751_init(void) +{ + return i2c_add_driver(&s5m8751_driver); +} +module_init(s5m8751_init); + +static void __exit s5m8751_exit(void) +{ + i2c_del_driver(&s5m8751_driver); +} +module_exit(s5m8751_exit); + +MODULE_DESCRIPTION("I2C Driver for Samsung S5M8751"); +MODULE_AUTHOR("Sangbeom Kim <sbkim73@samsung.com>"); +MODULE_LICENSE("GPL"); -- 1.7.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] mfd: Add I2C control support for S5M8751 2011-06-22 5:53 ` [PATCH 3/3] mfd: Add I2C control support for S5M8751 Sangbeom Kim @ 2011-06-22 8:56 ` Maxin B John 2011-06-22 12:50 ` Mark Brown 1 sibling, 0 replies; 16+ messages in thread From: Maxin B John @ 2011-06-22 8:56 UTC (permalink / raw) To: Sangbeom Kim; +Cc: sameo, linux-kernel Hi, On Wed, Jun 22, 2011 at 6:53 AM, Sangbeom Kim <sbkim73@samsung.com> wrote: > Implement the I2C control interface for the S5M8751. > > Signed-off-by: Sangbeom Kim <sbkim73@samsung.com> > --- > drivers/mfd/Kconfig | 9 +++ > drivers/mfd/Makefile | 2 + > drivers/mfd/s5m8751-i2c.c | 141 +++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 152 insertions(+), 0 deletions(-) > create mode 100644 drivers/mfd/s5m8751-i2c.c > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index 85c91ac..4a8468c 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -388,6 +388,15 @@ config MFD_WM831X_SPI > config MFD_S5M8751 > tristate > > +config MFD_S5M8751_I2C > + tristate "Support Samsung S5M8751 PMIC/Audio DAC with I2C" > + select MFD_S5M8751 > + select MFD_CORE > + depends on I2C > + help > + S5M8751 is an advanced power management with AUDIO DAC. This option > + enables chip support for the S5M8751 with I2C as the control interface. > + > config MFD_WM8350 > bool > depends on GENERIC_HARDIRQS > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index 27f99bb..b94476e 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -96,3 +96,5 @@ obj-$(CONFIG_MFD_PM8XXX_IRQ) += pm8xxx-irq.o > obj-$(CONFIG_MFD_TPS65910) += tps65910.o tps65910-irq.o > s5m8751-objs += s5m8751-core.o > obj-$(CONFIG_MFD_S5M8751) += s5m8751.o > +obj-$(CONFIG_MFD_S5M8751_I2C) += s5m8751-i2c.o > + > diff --git a/drivers/mfd/s5m8751-i2c.c b/drivers/mfd/s5m8751-i2c.c > new file mode 100644 > index 0000000..4ca680b > --- /dev/null > +++ b/drivers/mfd/s5m8751-i2c.c > @@ -0,0 +1,141 @@ > +/* linux/drivers/mfd/s5m8751-i2s.c > + * > + * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd. > + * http://www.samsung.com > + * > + * I2C driver for S5M8751 > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > +*/ > + > +#include <linux/module.h> > +#include <linux/moduleparam.h> > +#include <linux/init.h> > +#include <linux/interrupt.h> > +#include <linux/platform_device.h> > +#include <linux/i2c.h> > +#include <linux/mfd/s5m8751.h> > +#include <linux/slab.h> > + > +static int s5m8751_i2c_read_device(struct s5m8751 *s5m8751, uint8_t reg, > + uint8_t *val) > +{ > + int ret; > + ret = i2c_smbus_read_byte_data(s5m8751->i2c_client, reg); > + if (ret < 0) { > + dev_err(s5m8751->dev, "failed reading at 0x%02x\n", reg); > + return ret; > + } > + *val = (uint8_t)ret; > + return 0; > +} > + > +static int s5m8751_i2c_write_device(struct s5m8751 *s5m8751, uint8_t reg, > + uint8_t val) > +{ > + int ret; > + ret = i2c_smbus_write_byte_data(s5m8751->i2c_client, reg, val); > + if (ret < 0) { > + dev_err(s5m8751->dev, "failed writing 0x%02x to 0x%02x\n", > + val, reg); > + return ret; > + } > + return 0; > +} > + > +static int s5m8751_i2c_write_block_device(struct s5m8751 *s5m8751, uint8_t reg, > + int len, uint8_t *val) > +{ > + int ret; > + ret = i2c_smbus_write_i2c_block_data(s5m8751->i2c_client, reg, len, > + val); > + if (ret < 0) { > + dev_err(s5m8751->dev, "failed writings to 0x%02x\n", reg); > + return ret; > + } > + return 0; > +} > + > +static int s5m8751_i2c_read_block_device(struct s5m8751 *s5m8751, uint8_t reg, > + int len, uint8_t *val) > +{ > + int ret; > + ret = i2c_smbus_read_i2c_block_data(s5m8751->i2c_client, reg, len, val); > + if (ret < 0) { > + dev_err(s5m8751->dev, "failed reading from 0x%02x\n", reg); > + return ret; > + } > + return 0; > +} > + > +static int s5m8751_probe(struct i2c_client *i2c, > + const struct i2c_device_id *id) > +{ > + struct s5m8751 *s5m8751; > + int ret = 0; > + > + s5m8751 = kzalloc(sizeof(struct s5m8751), GFP_KERNEL); > + if (s5m8751 == NULL) > + goto err; Just my 2 cents. I think we should return -ENOMEM here. > + > + i2c_set_clientdata(i2c, s5m8751); > + > + s5m8751->dev = &i2c->dev; > + s5m8751->i2c_client = i2c; > + s5m8751->read_dev = s5m8751_i2c_read_device; > + s5m8751->write_dev = s5m8751_i2c_write_device; > + s5m8751->read_block_dev = s5m8751_i2c_read_block_device; > + s5m8751->write_block_dev = s5m8751_i2c_write_block_device; > + > + ret = s5m8751_device_init(s5m8751, i2c->irq, i2c->dev.platform_data); > + if (ret < 0) > + goto err; > + > + return 0; > + > +err: > + return ret; > +} > + > +static int s5m8751_remove(struct i2c_client *i2c) > +{ > + struct s5m8751 *s5m8751 = i2c_get_clientdata(i2c); > + > + s5m8751_device_exit(s5m8751); I think we should free the s5m8751 here to avoid memory leaks. > + > + return 0; > +} > + > +static const struct i2c_device_id s5m8751_i2c_id[] = { > + { "s5m8751", 0 }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, s5m8751_i2c_id); > + > +static struct i2c_driver s5m8751_driver = { > + .driver = { > + .name = "s5m8751", > + .owner = THIS_MODULE, > + }, > + .probe = s5m8751_probe, > + .remove = s5m8751_remove, > + .id_table = s5m8751_i2c_id, > +}; > + > +static int __init s5m8751_init(void) > +{ > + return i2c_add_driver(&s5m8751_driver); > +} > +module_init(s5m8751_init); > + > +static void __exit s5m8751_exit(void) > +{ > + i2c_del_driver(&s5m8751_driver); > +} > +module_exit(s5m8751_exit); > + > +MODULE_DESCRIPTION("I2C Driver for Samsung S5M8751"); > +MODULE_AUTHOR("Sangbeom Kim <sbkim73@samsung.com>"); > +MODULE_LICENSE("GPL"); > -- > 1.7.1 > > -- > 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/ > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] mfd: Add I2C control support for S5M8751 2011-06-22 5:53 ` [PATCH 3/3] mfd: Add I2C control support for S5M8751 Sangbeom Kim 2011-06-22 8:56 ` Maxin B John @ 2011-06-22 12:50 ` Mark Brown 2011-06-23 1:25 ` Sangbeom Kim 1 sibling, 1 reply; 16+ messages in thread From: Mark Brown @ 2011-06-22 12:50 UTC (permalink / raw) To: Sangbeom Kim; +Cc: sameo, linux-kernel On Wed, Jun 22, 2011 at 02:53:57PM +0900, Sangbeom Kim wrote: > Implement the I2C control interface for the S5M8751. > > Signed-off-by: Sangbeom Kim <sbkim73@samsung.com> If the device doesn't support SPI this could just as well be merged into the main driver - the reason for splitting with devices that support both is that it gives more flexibility with what gets built into the core kernel when both APIs are enabled. > +static int s5m8751_i2c_read_device(struct s5m8751 *s5m8751, uint8_t reg, > + uint8_t *val) > +{ > + int ret; > + ret = i2c_smbus_read_byte_data(s5m8751->i2c_client, reg); > + if (ret < 0) { > + dev_err(s5m8751->dev, "failed reading at 0x%02x\n", reg); > + return ret; > + } > + *val = (uint8_t)ret; Why is this case required? ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 3/3] mfd: Add I2C control support for S5M8751 2011-06-22 12:50 ` Mark Brown @ 2011-06-23 1:25 ` Sangbeom Kim 2011-06-23 1:28 ` Mark Brown 0 siblings, 1 reply; 16+ messages in thread From: Sangbeom Kim @ 2011-06-23 1:25 UTC (permalink / raw) To: 'Mark Brown'; +Cc: sameo, linux-kernel On Wed, Jun 22, 2011 at 09:51:57PM +0900, Mark Brown wrote: > If the device doesn't support SPI this could just as well be merged into > the main driver - the reason for splitting with devices that support > both is that it gives more flexibility with what gets built into the > core kernel when both APIs are enabled. Thank for your comment, We have a few PMIC (S5M8751, S5M8752 and others). They have different features. And Each one will have separate core file. But i2c interface is same. So, I try to separate i2s code. I would modify prefix like s5m87xx. > > +static int s5m8751_i2c_read_device(struct s5m8751 *s5m8751, uint8_t reg, > > + uint8_t *val) > > +{ > > + int ret; > > + ret = i2c_smbus_read_byte_data(s5m8751->i2c_client, reg); > > + if (ret < 0) { > > + dev_err(s5m8751->dev, "failed reading at 0x%02x\n", reg); > > + return ret; > > + } > > + *val = (uint8_t)ret; > > Why is this case required? I want pass the read value by val Thanks and regards, SB Kim ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] mfd: Add I2C control support for S5M8751 2011-06-23 1:25 ` Sangbeom Kim @ 2011-06-23 1:28 ` Mark Brown 2011-06-23 2:21 ` Sangbeom Kim 0 siblings, 1 reply; 16+ messages in thread From: Mark Brown @ 2011-06-23 1:28 UTC (permalink / raw) To: Sangbeom Kim; +Cc: sameo, linux-kernel On Thu, Jun 23, 2011 at 10:25:37AM +0900, Sangbeom Kim wrote: > Thank for your comment, > We have a few PMIC (S5M8751, S5M8752 and others). > They have different features. And Each one will have separate core file. > But i2c interface is same. > So, I try to separate i2s code. > I would modify prefix like s5m87xx. In that case you should definitely take a look at the regmap API I've been posting. It factors all the device I/O stuff out so you should be able to just tell it that you have a device with n bit registers and n bit values and it'll provide all the I/O functions you need. I just reposted it today, though it needs another round for some SPI stuff. > > > +static int s5m8751_i2c_read_device(struct s5m8751 *s5m8751, uint8_t > reg, > > > + uint8_t *val) > > > +{ > > > + int ret; > > > + ret = i2c_smbus_read_byte_data(s5m8751->i2c_client, reg); > > > + if (ret < 0) { > > > + dev_err(s5m8751->dev, "failed reading at 0x%02x\n", reg); > > > + return ret; > > > + } > > > + *val = (uint8_t)ret; > > Why is this case required? > I want pass the read value by val Sorry, typo - why is this *cast* required? ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 3/3] mfd: Add I2C control support for S5M8751 2011-06-23 1:28 ` Mark Brown @ 2011-06-23 2:21 ` Sangbeom Kim 2011-06-23 10:35 ` Mark Brown 0 siblings, 1 reply; 16+ messages in thread From: Sangbeom Kim @ 2011-06-23 2:21 UTC (permalink / raw) To: 'Mark Brown'; +Cc: sameo, linux-kernel On Thu, Jun 23, 2011 at 10:29 AM +0900, Mark Brown wrote: > > Thank for your comment, > > We have a few PMIC (S5M8751, S5M8752 and others). > > They have different features. And Each one will have separate core file. > > But i2c interface is same. > > So, I try to separate i2s code. > > I would modify prefix like s5m87xx. > > In that case you should definitely take a look at the regmap API I've > been posting. It factors all the device I/O stuff out so you should be > able to just tell it that you have a device with n bit registers and n > bit values and it'll provide all the I/O functions you need. I just > reposted it today, though it needs another round for some SPI stuff. OK, After reviewing your patch, I will apply it. > > > > +static int s5m8751_i2c_read_device(struct s5m8751 *s5m8751, uint8_t > > reg, > > > > + uint8_t *val) > > > > +{ > > > > + int ret; > > > > + ret = i2c_smbus_read_byte_data(s5m8751->i2c_client, reg); > > > > + if (ret < 0) { > > > > + dev_err(s5m8751->dev, "failed reading at 0x%02x\n", > reg); > > > > + return ret; > > > > + } > > > > + *val = (uint8_t)ret; > > > > Why is this case required? > > > I want pass the read value by val > > Sorry, typo - why is this *cast* required? Just want to make same type. Thanks, SB Kim ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] mfd: Add I2C control support for S5M8751 2011-06-23 2:21 ` Sangbeom Kim @ 2011-06-23 10:35 ` Mark Brown 0 siblings, 0 replies; 16+ messages in thread From: Mark Brown @ 2011-06-23 10:35 UTC (permalink / raw) To: Sangbeom Kim; +Cc: sameo, linux-kernel On Thu, Jun 23, 2011 at 11:21:10AM +0900, Sangbeom Kim wrote: > On Thu, Jun 23, 2011 at 10:29 AM +0900, Mark Brown wrote: > > In that case you should definitely take a look at the regmap API I've > > been posting. It factors all the device I/O stuff out so you should be > > able to just tell it that you have a device with n bit registers and n > > bit values and it'll provide all the I/O functions you need. I just > > reposted it today, though it needs another round for some SPI stuff. > OK, After reviewing your patch, I will apply it. Note that it's not actually in mainline yet, it's still being reviewed. > > > > > + *val = (uint8_t)ret; > > > > Why is this case required? > > > I want pass the read value by val > > Sorry, typo - why is this *cast* required? > Just want to make same type. Usually unless there's a reason to do a cast it's better to let the compiler figure things out as a cast can stop the toolchain identifying real issues for you. ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2011-07-04 23:49 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-06-22 5:53 [PATCH 0/3] S5M8751 core driver Sangbeom Kim 2011-06-22 5:53 ` [PATCH 1/3] mfd: Add S5M8751 register definitions Sangbeom Kim 2011-06-22 12:42 ` Mark Brown 2011-06-22 23:59 ` Sangbeom Kim 2011-06-22 5:53 ` [PATCH 2/3] mfd: Add initial S5M8751 support Sangbeom Kim 2011-06-22 12:48 ` Mark Brown 2011-06-23 1:14 ` Sangbeom Kim 2011-07-04 14:07 ` Samuel Ortiz 2011-07-04 23:49 ` Sangbeom Kim 2011-06-22 5:53 ` [PATCH 3/3] mfd: Add I2C control support for S5M8751 Sangbeom Kim 2011-06-22 8:56 ` Maxin B John 2011-06-22 12:50 ` Mark Brown 2011-06-23 1:25 ` Sangbeom Kim 2011-06-23 1:28 ` Mark Brown 2011-06-23 2:21 ` Sangbeom Kim 2011-06-23 10:35 ` Mark Brown
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.