* [PATCH v2 1/3] mfd: rtsx: reduce code duplication in rtl8411
2013-12-03 9:26 [PATCH v2 0/3] mfd: rtsx: decrease driver size and add new device micky_ching
@ 2013-12-03 9:26 ` micky_ching
2013-12-03 10:55 ` Lee Jones
2013-12-03 9:26 ` [PATCH v2 2/3] mfd: rtsx: add card reader rtl8402 micky_ching
2013-12-03 9:26 ` [PATCH v2 3/3] mfd: rtsx: prevent 'used uninitialised' warnings micky_ching
2 siblings, 1 reply; 15+ messages in thread
From: micky_ching @ 2013-12-03 9:26 UTC (permalink / raw)
To: sameo
Cc: devel, linux-kernel, gregkh, wei_wang, rogerable, Micky Ching,
Lee Jones
From: Micky Ching <micky_ching@realsil.com.cn>
in order to remove duplicated code in rtl8411, we make 8411 as the base
init params, and other like-8411 chips will just change the different
value with 8411, this can save some source code.
Signed-off-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Micky Ching <micky_ching@realsil.com.cn>
---
drivers/mfd/rtl8411.c | 63 +++++++++---------------------------------------
drivers/mfd/rtsx_pcr.h | 8 ++++++
2 files changed, 19 insertions(+), 52 deletions(-)
diff --git a/drivers/mfd/rtl8411.c b/drivers/mfd/rtl8411.c
index 5280135..c9eab9d 100644
--- a/drivers/mfd/rtl8411.c
+++ b/drivers/mfd/rtl8411.c
@@ -279,7 +279,7 @@ static int rtl8411_conv_clk_and_div_n(int input, int dir)
return output;
}
-static const struct pcr_ops rtl8411_pcr_ops = {
+static struct pcr_ops rtl8411_pcr_ops = {
.fetch_vendor_settings = rtl8411_fetch_vendor_settings,
.extra_init_hw = rtl8411_extra_init_hw,
.optimize_phy = NULL,
@@ -295,22 +295,6 @@ static const struct pcr_ops rtl8411_pcr_ops = {
.force_power_down = rtl8411_force_power_down,
};
-static const struct pcr_ops rtl8411b_pcr_ops = {
- .fetch_vendor_settings = rtl8411b_fetch_vendor_settings,
- .extra_init_hw = rtl8411b_extra_init_hw,
- .optimize_phy = NULL,
- .turn_on_led = rtl8411_turn_on_led,
- .turn_off_led = rtl8411_turn_off_led,
- .enable_auto_blink = rtl8411_enable_auto_blink,
- .disable_auto_blink = rtl8411_disable_auto_blink,
- .card_power_on = rtl8411_card_power_on,
- .card_power_off = rtl8411_card_power_off,
- .switch_output_voltage = rtl8411_switch_output_voltage,
- .cd_deglitch = rtl8411_cd_deglitch,
- .conv_clk_and_div_n = rtl8411_conv_clk_and_div_n,
- .force_power_down = rtl8411_force_power_down,
-};
-
/* SD Pull Control Enable:
* SD_DAT[3:0] ==> pull up
* SD_CD ==> pull up
@@ -456,45 +440,20 @@ void rtl8411_init_params(struct rtsx_pcr *pcr)
pcr->rx_initial_phase = SET_CLOCK_PHASE(4, 3, 10);
pcr->ic_version = rtl8411_get_ic_version(pcr);
- pcr->sd_pull_ctl_enable_tbl = rtl8411_sd_pull_ctl_enable_tbl;
- pcr->sd_pull_ctl_disable_tbl = rtl8411_sd_pull_ctl_disable_tbl;
- pcr->ms_pull_ctl_enable_tbl = rtl8411_ms_pull_ctl_enable_tbl;
- pcr->ms_pull_ctl_disable_tbl = rtl8411_ms_pull_ctl_disable_tbl;
+
+ set_pull_ctrl_tables(rtl8411);
}
void rtl8411b_init_params(struct rtsx_pcr *pcr)
{
- pcr->extra_caps = EXTRA_CAPS_SD_SDR50 | EXTRA_CAPS_SD_SDR104;
- pcr->num_slots = 2;
- pcr->ops = &rtl8411b_pcr_ops;
-
- pcr->flags = 0;
- pcr->card_drive_sel = RTL8411_CARD_DRIVE_DEFAULT;
- pcr->sd30_drive_sel_1v8 = DRIVER_TYPE_B;
- pcr->sd30_drive_sel_3v3 = DRIVER_TYPE_D;
- pcr->aspm_en = ASPM_L1_EN;
- pcr->tx_initial_phase = SET_CLOCK_PHASE(23, 7, 14);
- pcr->rx_initial_phase = SET_CLOCK_PHASE(4, 3, 10);
+ rtl8411_init_params(pcr);
- pcr->ic_version = rtl8411_get_ic_version(pcr);
+ rtl8411_pcr_ops.fetch_vendor_settings =
+ rtl8411b_fetch_vendor_settings;
+ rtl8411_pcr_ops.extra_init_hw = rtl8411b_extra_init_hw;
- if (rtl8411b_is_qfn48(pcr)) {
- pcr->sd_pull_ctl_enable_tbl =
- rtl8411b_qfn48_sd_pull_ctl_enable_tbl;
- pcr->sd_pull_ctl_disable_tbl =
- rtl8411b_qfn48_sd_pull_ctl_disable_tbl;
- pcr->ms_pull_ctl_enable_tbl =
- rtl8411b_qfn48_ms_pull_ctl_enable_tbl;
- pcr->ms_pull_ctl_disable_tbl =
- rtl8411b_qfn48_ms_pull_ctl_disable_tbl;
- } else {
- pcr->sd_pull_ctl_enable_tbl =
- rtl8411b_qfn64_sd_pull_ctl_enable_tbl;
- pcr->sd_pull_ctl_disable_tbl =
- rtl8411b_qfn64_sd_pull_ctl_disable_tbl;
- pcr->ms_pull_ctl_enable_tbl =
- rtl8411b_qfn64_ms_pull_ctl_enable_tbl;
- pcr->ms_pull_ctl_disable_tbl =
- rtl8411b_qfn64_ms_pull_ctl_disable_tbl;
- }
+ if (rtl8411b_is_qfn48(pcr))
+ set_pull_ctrl_tables(rtl8411b_qfn48);
+ else
+ set_pull_ctrl_tables(rtl8411b_qfn64);
}
diff --git a/drivers/mfd/rtsx_pcr.h b/drivers/mfd/rtsx_pcr.h
index 947e79b..26b52ec 100644
--- a/drivers/mfd/rtsx_pcr.h
+++ b/drivers/mfd/rtsx_pcr.h
@@ -63,4 +63,12 @@ static inline u8 map_sd_drive(int idx)
#define rtl8411_reg_to_sd30_drive_sel_3v3(reg) (((reg) >> 5) & 0x07)
#define rtl8411b_reg_to_sd30_drive_sel_3v3(reg) ((reg) & 0x03)
+#define set_pull_ctrl_tables(__device) \
+do { \
+ pcr->sd_pull_ctl_enable_tbl = __device##_sd_pull_ctl_enable_tbl; \
+ pcr->sd_pull_ctl_disable_tbl = __device##_sd_pull_ctl_disable_tbl; \
+ pcr->ms_pull_ctl_enable_tbl = __device##_ms_pull_ctl_enable_tbl; \
+ pcr->ms_pull_ctl_disable_tbl = __device##_ms_pull_ctl_disable_tbl; \
+} while (0)
+
#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 1/3] mfd: rtsx: reduce code duplication in rtl8411
2013-12-03 9:26 ` [PATCH v2 1/3] mfd: rtsx: reduce code duplication in rtl8411 micky_ching
@ 2013-12-03 10:55 ` Lee Jones
0 siblings, 0 replies; 15+ messages in thread
From: Lee Jones @ 2013-12-03 10:55 UTC (permalink / raw)
To: micky_ching; +Cc: sameo, devel, linux-kernel, gregkh, wei_wang, rogerable
> From: Micky Ching <micky_ching@realsil.com.cn>
>
> in order to remove duplicated code in rtl8411, we make 8411 as the base
> init params, and other like-8411 chips will just change the different
> value with 8411, this can save some source code.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
I haven't signed this version off.
I'm still confused what the difference between the versions are. I
submitted a patch just like this one, which you responded to with only
a single fix. Then you've submitted a slightly different version? What
makes this version different/better?
> Signed-off-by: Micky Ching <micky_ching@realsil.com.cn>
> ---
> drivers/mfd/rtl8411.c | 63 +++++++++---------------------------------------
> drivers/mfd/rtsx_pcr.h | 8 ++++++
> 2 files changed, 19 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/mfd/rtl8411.c b/drivers/mfd/rtl8411.c
> index 5280135..c9eab9d 100644
> --- a/drivers/mfd/rtl8411.c
> +++ b/drivers/mfd/rtl8411.c
> @@ -279,7 +279,7 @@ static int rtl8411_conv_clk_and_div_n(int input, int dir)
> return output;
> }
>
> -static const struct pcr_ops rtl8411_pcr_ops = {
> +static struct pcr_ops rtl8411_pcr_ops = {
> .fetch_vendor_settings = rtl8411_fetch_vendor_settings,
> .extra_init_hw = rtl8411_extra_init_hw,
> .optimize_phy = NULL,
> @@ -295,22 +295,6 @@ static const struct pcr_ops rtl8411_pcr_ops = {
> .force_power_down = rtl8411_force_power_down,
> };
>
> -static const struct pcr_ops rtl8411b_pcr_ops = {
> - .fetch_vendor_settings = rtl8411b_fetch_vendor_settings,
> - .extra_init_hw = rtl8411b_extra_init_hw,
> - .optimize_phy = NULL,
> - .turn_on_led = rtl8411_turn_on_led,
> - .turn_off_led = rtl8411_turn_off_led,
> - .enable_auto_blink = rtl8411_enable_auto_blink,
> - .disable_auto_blink = rtl8411_disable_auto_blink,
> - .card_power_on = rtl8411_card_power_on,
> - .card_power_off = rtl8411_card_power_off,
> - .switch_output_voltage = rtl8411_switch_output_voltage,
> - .cd_deglitch = rtl8411_cd_deglitch,
> - .conv_clk_and_div_n = rtl8411_conv_clk_and_div_n,
> - .force_power_down = rtl8411_force_power_down,
> -};
> -
> /* SD Pull Control Enable:
> * SD_DAT[3:0] ==> pull up
> * SD_CD ==> pull up
> @@ -456,45 +440,20 @@ void rtl8411_init_params(struct rtsx_pcr *pcr)
> pcr->rx_initial_phase = SET_CLOCK_PHASE(4, 3, 10);
>
> pcr->ic_version = rtl8411_get_ic_version(pcr);
> - pcr->sd_pull_ctl_enable_tbl = rtl8411_sd_pull_ctl_enable_tbl;
> - pcr->sd_pull_ctl_disable_tbl = rtl8411_sd_pull_ctl_disable_tbl;
> - pcr->ms_pull_ctl_enable_tbl = rtl8411_ms_pull_ctl_enable_tbl;
> - pcr->ms_pull_ctl_disable_tbl = rtl8411_ms_pull_ctl_disable_tbl;
> +
> + set_pull_ctrl_tables(rtl8411);
> }
>
> void rtl8411b_init_params(struct rtsx_pcr *pcr)
> {
> - pcr->extra_caps = EXTRA_CAPS_SD_SDR50 | EXTRA_CAPS_SD_SDR104;
> - pcr->num_slots = 2;
> - pcr->ops = &rtl8411b_pcr_ops;
> -
> - pcr->flags = 0;
> - pcr->card_drive_sel = RTL8411_CARD_DRIVE_DEFAULT;
> - pcr->sd30_drive_sel_1v8 = DRIVER_TYPE_B;
> - pcr->sd30_drive_sel_3v3 = DRIVER_TYPE_D;
> - pcr->aspm_en = ASPM_L1_EN;
> - pcr->tx_initial_phase = SET_CLOCK_PHASE(23, 7, 14);
> - pcr->rx_initial_phase = SET_CLOCK_PHASE(4, 3, 10);
> + rtl8411_init_params(pcr);
>
> - pcr->ic_version = rtl8411_get_ic_version(pcr);
> + rtl8411_pcr_ops.fetch_vendor_settings =
> + rtl8411b_fetch_vendor_settings;
> + rtl8411_pcr_ops.extra_init_hw = rtl8411b_extra_init_hw;
>
> - if (rtl8411b_is_qfn48(pcr)) {
> - pcr->sd_pull_ctl_enable_tbl =
> - rtl8411b_qfn48_sd_pull_ctl_enable_tbl;
> - pcr->sd_pull_ctl_disable_tbl =
> - rtl8411b_qfn48_sd_pull_ctl_disable_tbl;
> - pcr->ms_pull_ctl_enable_tbl =
> - rtl8411b_qfn48_ms_pull_ctl_enable_tbl;
> - pcr->ms_pull_ctl_disable_tbl =
> - rtl8411b_qfn48_ms_pull_ctl_disable_tbl;
> - } else {
> - pcr->sd_pull_ctl_enable_tbl =
> - rtl8411b_qfn64_sd_pull_ctl_enable_tbl;
> - pcr->sd_pull_ctl_disable_tbl =
> - rtl8411b_qfn64_sd_pull_ctl_disable_tbl;
> - pcr->ms_pull_ctl_enable_tbl =
> - rtl8411b_qfn64_ms_pull_ctl_enable_tbl;
> - pcr->ms_pull_ctl_disable_tbl =
> - rtl8411b_qfn64_ms_pull_ctl_disable_tbl;
> - }
> + if (rtl8411b_is_qfn48(pcr))
> + set_pull_ctrl_tables(rtl8411b_qfn48);
> + else
> + set_pull_ctrl_tables(rtl8411b_qfn64);
> }
> diff --git a/drivers/mfd/rtsx_pcr.h b/drivers/mfd/rtsx_pcr.h
> index 947e79b..26b52ec 100644
> --- a/drivers/mfd/rtsx_pcr.h
> +++ b/drivers/mfd/rtsx_pcr.h
> @@ -63,4 +63,12 @@ static inline u8 map_sd_drive(int idx)
> #define rtl8411_reg_to_sd30_drive_sel_3v3(reg) (((reg) >> 5) & 0x07)
> #define rtl8411b_reg_to_sd30_drive_sel_3v3(reg) ((reg) & 0x03)
>
> +#define set_pull_ctrl_tables(__device) \
> +do { \
> + pcr->sd_pull_ctl_enable_tbl = __device##_sd_pull_ctl_enable_tbl; \
> + pcr->sd_pull_ctl_disable_tbl = __device##_sd_pull_ctl_disable_tbl; \
> + pcr->ms_pull_ctl_enable_tbl = __device##_ms_pull_ctl_enable_tbl; \
> + pcr->ms_pull_ctl_disable_tbl = __device##_ms_pull_ctl_disable_tbl; \
> +} while (0)
> +
> #endif
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/3] mfd: rtsx: add card reader rtl8402
2013-12-03 9:26 [PATCH v2 0/3] mfd: rtsx: decrease driver size and add new device micky_ching
2013-12-03 9:26 ` [PATCH v2 1/3] mfd: rtsx: reduce code duplication in rtl8411 micky_ching
@ 2013-12-03 9:26 ` micky_ching
2013-12-03 10:45 ` Lee Jones
2013-12-03 9:26 ` [PATCH v2 3/3] mfd: rtsx: prevent 'used uninitialised' warnings micky_ching
2 siblings, 1 reply; 15+ messages in thread
From: micky_ching @ 2013-12-03 9:26 UTC (permalink / raw)
To: sameo
Cc: devel, linux-kernel, gregkh, wei_wang, rogerable, Micky Ching,
Lee Jones
From: Micky Ching <micky_ching@realsil.com.cn>
Add card reader rtl8042, rtl8402 is much like rtl8411, so just add it to
rtl8411.c
Signed-off-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Micky Ching <micky_ching@realsil.com.cn>
---
drivers/mfd/rtl8411.c | 33 +++++++++++++++++++++++++++++++++
drivers/mfd/rtsx_pcr.c | 5 +++++
drivers/mfd/rtsx_pcr.h | 1 +
3 files changed, 39 insertions(+)
diff --git a/drivers/mfd/rtl8411.c b/drivers/mfd/rtl8411.c
index c9eab9d..4b377e3 100644
--- a/drivers/mfd/rtl8411.c
+++ b/drivers/mfd/rtl8411.c
@@ -216,6 +216,31 @@ static int rtl8411_switch_output_voltage(struct rtsx_pcr *pcr, u8 voltage)
return rtsx_pci_write_register(pcr, LDO_CTL, mask, val);
}
+static int rtl8402_switch_output_voltage(struct rtsx_pcr *pcr, u8 voltage)
+{
+ u8 mask, val;
+ int err;
+
+ mask = (BPP_REG_TUNED18 << BPP_TUNED18_SHIFT_8402) | BPP_PAD_MASK;
+ if (voltage == OUTPUT_3V3) {
+ err = rtsx_pci_write_register(pcr,
+ SD30_DRIVE_SEL, 0x07, pcr->sd30_drive_sel_3v3);
+ if (err < 0)
+ return err;
+ val = (BPP_ASIC_3V3 << BPP_TUNED18_SHIFT_8402) | BPP_PAD_3V3;
+ } else if (voltage == OUTPUT_1V8) {
+ err = rtsx_pci_write_register(pcr,
+ SD30_DRIVE_SEL, 0x07, pcr->sd30_drive_sel_1v8);
+ if (err < 0)
+ return err;
+ val = (BPP_ASIC_2V0 << BPP_TUNED18_SHIFT_8402) | BPP_PAD_1V8;
+ } else {
+ return -EINVAL;
+ }
+
+ return rtsx_pci_write_register(pcr, LDO_CTL, mask, val);
+}
+
static unsigned int rtl8411_cd_deglitch(struct rtsx_pcr *pcr)
{
unsigned int card_exist;
@@ -457,3 +482,11 @@ void rtl8411b_init_params(struct rtsx_pcr *pcr)
else
set_pull_ctrl_tables(rtl8411b_qfn64);
}
+
+void rtl8402_init_params(struct rtsx_pcr *pcr)
+{
+ rtl8411_init_params(pcr);
+
+ rtl8411_pcr_ops.switch_output_voltage =
+ rtl8402_switch_output_voltage;
+}
diff --git a/drivers/mfd/rtsx_pcr.c b/drivers/mfd/rtsx_pcr.c
index 11e20af..2af55bb 100644
--- a/drivers/mfd/rtsx_pcr.c
+++ b/drivers/mfd/rtsx_pcr.c
@@ -57,6 +57,7 @@ static DEFINE_PCI_DEVICE_TABLE(rtsx_pci_ids) = {
{ PCI_DEVICE(0x10EC, 0x5227), PCI_CLASS_OTHERS << 16, 0xFF0000 },
{ PCI_DEVICE(0x10EC, 0x5249), PCI_CLASS_OTHERS << 16, 0xFF0000 },
{ PCI_DEVICE(0x10EC, 0x5287), PCI_CLASS_OTHERS << 16, 0xFF0000 },
+ { PCI_DEVICE(0x10EC, 0x5286), PCI_CLASS_OTHERS << 16, 0xFF0000 },
{ 0, }
};
@@ -1061,6 +1062,10 @@ static int rtsx_pci_init_chip(struct rtsx_pcr *pcr)
case 0x5287:
rtl8411b_init_params(pcr);
break;
+
+ case 0x5286:
+ rtl8402_init_params(pcr);
+ break;
}
dev_dbg(&(pcr->pci->dev), "PID: 0x%04x, IC version: 0x%02x\n",
diff --git a/drivers/mfd/rtsx_pcr.h b/drivers/mfd/rtsx_pcr.h
index 26b52ec..c097dbd 100644
--- a/drivers/mfd/rtsx_pcr.h
+++ b/drivers/mfd/rtsx_pcr.h
@@ -30,6 +30,7 @@
void rts5209_init_params(struct rtsx_pcr *pcr);
void rts5229_init_params(struct rtsx_pcr *pcr);
void rtl8411_init_params(struct rtsx_pcr *pcr);
+void rtl8402_init_params(struct rtsx_pcr *pcr);
void rts5227_init_params(struct rtsx_pcr *pcr);
void rts5249_init_params(struct rtsx_pcr *pcr);
void rtl8411b_init_params(struct rtsx_pcr *pcr);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 2/3] mfd: rtsx: add card reader rtl8402
2013-12-03 9:26 ` [PATCH v2 2/3] mfd: rtsx: add card reader rtl8402 micky_ching
@ 2013-12-03 10:45 ` Lee Jones
2013-12-03 11:02 ` micky
0 siblings, 1 reply; 15+ messages in thread
From: Lee Jones @ 2013-12-03 10:45 UTC (permalink / raw)
To: micky_ching; +Cc: sameo, devel, linux-kernel, gregkh, wei_wang, rogerable
> From: Micky Ching <micky_ching@realsil.com.cn>
>
> Add card reader rtl8042, rtl8402 is much like rtl8411, so just add it to
> rtl8411.c
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
I have never signed this patch off.
These tags actually 'mean' something. Please don't apply them willy-nilly!
> Signed-off-by: Micky Ching <micky_ching@realsil.com.cn>
> ---
> drivers/mfd/rtl8411.c | 33 +++++++++++++++++++++++++++++++++
> drivers/mfd/rtsx_pcr.c | 5 +++++
> drivers/mfd/rtsx_pcr.h | 1 +
> 3 files changed, 39 insertions(+)
>
> diff --git a/drivers/mfd/rtl8411.c b/drivers/mfd/rtl8411.c
> index c9eab9d..4b377e3 100644
> --- a/drivers/mfd/rtl8411.c
> +++ b/drivers/mfd/rtl8411.c
> @@ -216,6 +216,31 @@ static int rtl8411_switch_output_voltage(struct rtsx_pcr *pcr, u8 voltage)
> return rtsx_pci_write_register(pcr, LDO_CTL, mask, val);
> }
>
> +static int rtl8402_switch_output_voltage(struct rtsx_pcr *pcr, u8 voltage)
> +{
> + u8 mask, val;
> + int err;
> +
> + mask = (BPP_REG_TUNED18 << BPP_TUNED18_SHIFT_8402) | BPP_PAD_MASK;
> + if (voltage == OUTPUT_3V3) {
> + err = rtsx_pci_write_register(pcr,
> + SD30_DRIVE_SEL, 0x07, pcr->sd30_drive_sel_3v3);
> + if (err < 0)
> + return err;
> + val = (BPP_ASIC_3V3 << BPP_TUNED18_SHIFT_8402) | BPP_PAD_3V3;
> + } else if (voltage == OUTPUT_1V8) {
> + err = rtsx_pci_write_register(pcr,
> + SD30_DRIVE_SEL, 0x07, pcr->sd30_drive_sel_1v8);
> + if (err < 0)
> + return err;
> + val = (BPP_ASIC_2V0 << BPP_TUNED18_SHIFT_8402) | BPP_PAD_1V8;
> + } else {
> + return -EINVAL;
> + }
> +
> + return rtsx_pci_write_register(pcr, LDO_CTL, mask, val);
> +}
> +
I opposed this on last submission.
Please find a better way to do it.
Ideally, I'd like to see one function for this kind of thing. The
information required to ensure this is carried out correctly on each
differing platform can be passed in via a struct instead.
<snip>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 2/3] mfd: rtsx: add card reader rtl8402
2013-12-03 10:45 ` Lee Jones
@ 2013-12-03 11:02 ` micky
0 siblings, 0 replies; 15+ messages in thread
From: micky @ 2013-12-03 11:02 UTC (permalink / raw)
To: Lee Jones; +Cc: sameo, devel, linux-kernel, gregkh, wei_wang, rogerable
On 12/03/2013 06:45 PM, Lee Jones wrote:
>> From: Micky Ching<micky_ching@realsil.com.cn>
>>
>> Add card reader rtl8042, rtl8402 is much like rtl8411, so just add it to
>> rtl8411.c
>>
>> Signed-off-by: Lee Jones<lee.jones@linaro.org>
> I have never signed this patch off.
>
> These tags actually 'mean' something. Please don't apply them willy-nilly!
ok, I will remove it.
>> Signed-off-by: Micky Ching<micky_ching@realsil.com.cn>
>> ---
>> drivers/mfd/rtl8411.c | 33 +++++++++++++++++++++++++++++++++
>> drivers/mfd/rtsx_pcr.c | 5 +++++
>> drivers/mfd/rtsx_pcr.h | 1 +
>> 3 files changed, 39 insertions(+)
>>
>> diff --git a/drivers/mfd/rtl8411.c b/drivers/mfd/rtl8411.c
>> index c9eab9d..4b377e3 100644
>> --- a/drivers/mfd/rtl8411.c
>> +++ b/drivers/mfd/rtl8411.c
>> @@ -216,6 +216,31 @@ static int rtl8411_switch_output_voltage(struct rtsx_pcr *pcr, u8 voltage)
>> return rtsx_pci_write_register(pcr, LDO_CTL, mask, val);
>> }
>>
>> +static int rtl8402_switch_output_voltage(struct rtsx_pcr *pcr, u8 voltage)
>> +{
>> + u8 mask, val;
>> + int err;
>> +
>> + mask = (BPP_REG_TUNED18 << BPP_TUNED18_SHIFT_8402) | BPP_PAD_MASK;
>> + if (voltage == OUTPUT_3V3) {
>> + err = rtsx_pci_write_register(pcr,
>> + SD30_DRIVE_SEL, 0x07, pcr->sd30_drive_sel_3v3);
>> + if (err < 0)
>> + return err;
>> + val = (BPP_ASIC_3V3 << BPP_TUNED18_SHIFT_8402) | BPP_PAD_3V3;
>> + } else if (voltage == OUTPUT_1V8) {
>> + err = rtsx_pci_write_register(pcr,
>> + SD30_DRIVE_SEL, 0x07, pcr->sd30_drive_sel_1v8);
>> + if (err < 0)
>> + return err;
>> + val = (BPP_ASIC_2V0 << BPP_TUNED18_SHIFT_8402) | BPP_PAD_1V8;
>> + } else {
>> + return -EINVAL;
>> + }
>> +
>> + return rtsx_pci_write_register(pcr, LDO_CTL, mask, val);
>> +}
>> +
> I opposed this on last submission.
>
> Please find a better way to do it.
>
> Ideally, I'd like to see one function for this kind of thing. The
> information required to ensure this is carried out correctly on each
> differing platform can be passed in via a struct instead.
ok, I will add some member to rtsx_pcr to merge the function.
> <snip>
>
--
Best Regards
Micky.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 3/3] mfd: rtsx: prevent 'used uninitialised' warnings
2013-12-03 9:26 [PATCH v2 0/3] mfd: rtsx: decrease driver size and add new device micky_ching
2013-12-03 9:26 ` [PATCH v2 1/3] mfd: rtsx: reduce code duplication in rtl8411 micky_ching
2013-12-03 9:26 ` [PATCH v2 2/3] mfd: rtsx: add card reader rtl8402 micky_ching
@ 2013-12-03 9:26 ` micky_ching
2013-12-03 10:40 ` Lee Jones
2 siblings, 1 reply; 15+ messages in thread
From: micky_ching @ 2013-12-03 9:26 UTC (permalink / raw)
To: sameo
Cc: devel, linux-kernel, gregkh, wei_wang, rogerable, Micky Ching,
Lee Jones
From: Micky Ching <micky_ching@realsil.com.cn>
drivers/mfd/rtl8411.c: In function 'rtl8411_fetch_vendor_settings':
drivers/mfd/rtl8411.c:58:7: warning: 'reg1' is used uninitialized in this function [-Wuninitialized]
drivers/mfd/rtl8411.c: In function 'rtl8411b_fetch_vendor_settings':
drivers/mfd/rtl8411.c:79:7: warning: 'reg' is used uninitialized in this function [-Wuninitialized]
drivers/mfd/rtl8411.c: In function 'rtl8411_fetch_vendor_settings':
drivers/mfd/rtl8411.c:69:26: warning: 'reg3' may be used uninitialized in this function [-Wuninitialized]
Signed-off-by: Lee Jones <lee.jones@linaro.org>
Tested-by: Micky Ching <micky_ching@realsil.com.cn>
---
drivers/mfd/rtl8411.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mfd/rtl8411.c b/drivers/mfd/rtl8411.c
index 4b377e3..d53ff96 100644
--- a/drivers/mfd/rtl8411.c
+++ b/drivers/mfd/rtl8411.c
@@ -49,8 +49,8 @@ static int rtl8411b_is_qfn48(struct rtsx_pcr *pcr)
static void rtl8411_fetch_vendor_settings(struct rtsx_pcr *pcr)
{
- u32 reg1;
- u8 reg3;
+ u32 reg1 = 0;
+ u8 reg3 = 0;
rtsx_pci_read_config_dword(pcr, PCR_SETTING_REG1, ®1);
dev_dbg(&(pcr->pci->dev), "Cfg 0x%x: 0x%x\n", PCR_SETTING_REG1, reg1);
@@ -71,7 +71,7 @@ static void rtl8411_fetch_vendor_settings(struct rtsx_pcr *pcr)
static void rtl8411b_fetch_vendor_settings(struct rtsx_pcr *pcr)
{
- u32 reg;
+ u32 reg = 0;
rtsx_pci_read_config_dword(pcr, PCR_SETTING_REG1, ®);
dev_dbg(&(pcr->pci->dev), "Cfg 0x%x: 0x%x\n", PCR_SETTING_REG1, reg);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 3/3] mfd: rtsx: prevent 'used uninitialised' warnings
2013-12-03 9:26 ` [PATCH v2 3/3] mfd: rtsx: prevent 'used uninitialised' warnings micky_ching
@ 2013-12-03 10:40 ` Lee Jones
2013-12-03 10:40 ` Lee Jones
2013-12-03 10:54 ` micky
0 siblings, 2 replies; 15+ messages in thread
From: Lee Jones @ 2013-12-03 10:40 UTC (permalink / raw)
To: micky_ching; +Cc: sameo, devel, linux-kernel, gregkh, wei_wang, rogerable
On Tue, 03 Dec 2013, micky_ching@realsil.com.cn wrote:
> From: Micky Ching <micky_ching@realsil.com.cn>
This should contain the author's name.
You should use:
`git rebase -i` /* Use edit option */
then
`git commit --amend --author="First Last <first.last@email.com>"
To apply it.
> drivers/mfd/rtl8411.c: In function 'rtl8411_fetch_vendor_settings':
> drivers/mfd/rtl8411.c:58:7: warning: 'reg1' is used uninitialized in this function [-Wuninitialized]
> drivers/mfd/rtl8411.c: In function 'rtl8411b_fetch_vendor_settings':
> drivers/mfd/rtl8411.c:79:7: warning: 'reg' is used uninitialized in this function [-Wuninitialized]
> drivers/mfd/rtl8411.c: In function 'rtl8411_fetch_vendor_settings':
> drivers/mfd/rtl8411.c:69:26: warning: 'reg3' may be used uninitialized in this function [-Wuninitialized]
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> Tested-by: Micky Ching <micky_ching@realsil.com.cn>
I know this is a bit of a strange situation, but whenever you sent
patches to the MLs, you need to sign it off yourself too.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] mfd: rtsx: prevent 'used uninitialised' warnings
2013-12-03 10:40 ` Lee Jones
@ 2013-12-03 10:40 ` Lee Jones
2013-12-03 11:03 ` micky
2013-12-03 10:54 ` micky
1 sibling, 1 reply; 15+ messages in thread
From: Lee Jones @ 2013-12-03 10:40 UTC (permalink / raw)
To: micky_ching; +Cc: sameo, devel, linux-kernel, gregkh, wei_wang, rogerable
By the way, I'm going to apply this patch now.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] mfd: rtsx: prevent 'used uninitialised' warnings
2013-12-03 10:40 ` Lee Jones
@ 2013-12-03 11:03 ` micky
2013-12-03 11:28 ` Lee Jones
0 siblings, 1 reply; 15+ messages in thread
From: micky @ 2013-12-03 11:03 UTC (permalink / raw)
To: Lee Jones; +Cc: sameo, devel, linux-kernel, gregkh, wei_wang, rogerable
On 12/03/2013 06:40 PM, Lee Jones wrote:
> By the way, I'm going to apply this patch now.
>
So, in next patch-set, I will not include this patch.
--
Best Regards
Micky.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] mfd: rtsx: prevent 'used uninitialised' warnings
2013-12-03 11:03 ` micky
@ 2013-12-03 11:28 ` Lee Jones
0 siblings, 0 replies; 15+ messages in thread
From: Lee Jones @ 2013-12-03 11:28 UTC (permalink / raw)
To: micky; +Cc: sameo, devel, linux-kernel, gregkh, wei_wang, rogerable
On Tue, 03 Dec 2013, micky wrote:
> On 12/03/2013 06:40 PM, Lee Jones wrote:
> >By the way, I'm going to apply this patch now.
> >
> So, in next patch-set, I will not include this patch.
Exactly.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] mfd: rtsx: prevent 'used uninitialised' warnings
2013-12-03 10:40 ` Lee Jones
2013-12-03 10:40 ` Lee Jones
@ 2013-12-03 10:54 ` micky
2013-12-03 11:01 ` Lee Jones
2013-12-03 11:04 ` Dan Carpenter
1 sibling, 2 replies; 15+ messages in thread
From: micky @ 2013-12-03 10:54 UTC (permalink / raw)
To: Lee Jones; +Cc: sameo, devel, linux-kernel, gregkh, wei_wang, rogerable
On 12/03/2013 06:40 PM, Lee Jones wrote:
> This should contain the author's name.
>
> You should use:
> `git rebase -i` /* Use edit option */
> then
> `git commit --amend --author="First Last <first.last@email.com>"
>
> To apply it.
>
Sorry Lee, I will modify it. Is the order important? or only your name
is need here?
>> drivers/mfd/rtl8411.c: In function 'rtl8411_fetch_vendor_settings':
>> drivers/mfd/rtl8411.c:58:7: warning: 'reg1' is used uninitialized in this function [-Wuninitialized]
>> drivers/mfd/rtl8411.c: In function 'rtl8411b_fetch_vendor_settings':
>> drivers/mfd/rtl8411.c:79:7: warning: 'reg' is used uninitialized in this function [-Wuninitialized]
>> drivers/mfd/rtl8411.c: In function 'rtl8411_fetch_vendor_settings':
>> drivers/mfd/rtl8411.c:69:26: warning: 'reg3' may be used uninitialized in this function [-Wuninitialized]
>>
>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>> Tested-by: Micky Ching <micky_ching@realsil.com.cn>
> I know this is a bit of a strange situation, but whenever you sent
> patches to the MLs, you need to sign it off yourself too.
>
>
Did you mean I need write like this ?
Signed-off-by: Lee Jones <lee.jones@linaro.org>
Tested-by: Micky Ching <micky_ching@realsil.com.cn>
Signed-off-by: Micky Ching <micky_ching@realsil.com.cn>
--
Best Regards
Micky.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] mfd: rtsx: prevent 'used uninitialised' warnings
2013-12-03 10:54 ` micky
@ 2013-12-03 11:01 ` Lee Jones
2013-12-03 11:07 ` micky
2013-12-03 11:04 ` Dan Carpenter
1 sibling, 1 reply; 15+ messages in thread
From: Lee Jones @ 2013-12-03 11:01 UTC (permalink / raw)
To: micky; +Cc: sameo, devel, linux-kernel, gregkh, wei_wang, rogerable
> >This should contain the author's name.
> >
> >You should use:
> > `git rebase -i` /* Use edit option */
> >then
> > `git commit --amend --author="First Last <first.last@email.com>"
> >
> >To apply it.
> >
> Sorry Lee, I will modify it. Is the order important? or only your
> name is need here?
> >>drivers/mfd/rtl8411.c: In function 'rtl8411_fetch_vendor_settings':
> >>drivers/mfd/rtl8411.c:58:7: warning: 'reg1' is used uninitialized in this function [-Wuninitialized]
> >>drivers/mfd/rtl8411.c: In function 'rtl8411b_fetch_vendor_settings':
> >>drivers/mfd/rtl8411.c:79:7: warning: 'reg' is used uninitialized in this function [-Wuninitialized]
> >>drivers/mfd/rtl8411.c: In function 'rtl8411_fetch_vendor_settings':
> >>drivers/mfd/rtl8411.c:69:26: warning: 'reg3' may be used uninitialized in this function [-Wuninitialized]
> >>
> >>Signed-off-by: Lee Jones <lee.jones@linaro.org>
> >>Tested-by: Micky Ching <micky_ching@realsil.com.cn>
> >I know this is a bit of a strange situation, but whenever you sent
> >patches to the MLs, you need to sign it off yourself too.
Actually, you don't need to worry now. I already applied the patch
with the correct author applied with your Ack.
> Did you mean I need write like this ?
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> Tested-by: Micky Ching <micky_ching@realsil.com.cn>
> Signed-off-by: Micky Ching <micky_ching@realsil.com.cn>
As I say, this situation is a bit different, but in the normal case,
yes. Then the Maintainer will probably remove some and chop them
around a little to his/her liking.
FYI: The top SoB is usually the author, the subsequent ones are
usually gatekeepers of upstream repos. Anyone can take a patch and
submit it though, so in the case where the submitter isn't the author,
the second SoB is usually the patch submitter.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] mfd: rtsx: prevent 'used uninitialised' warnings
2013-12-03 11:01 ` Lee Jones
@ 2013-12-03 11:07 ` micky
0 siblings, 0 replies; 15+ messages in thread
From: micky @ 2013-12-03 11:07 UTC (permalink / raw)
To: Lee Jones; +Cc: sameo, devel, linux-kernel, gregkh, wei_wang, rogerable
On 12/03/2013 07:01 PM, Lee Jones wrote:
>>> This should contain the author's name.
>>>
>>> You should use:
>>> `git rebase -i` /* Use edit option */
>>> then
>>> `git commit --amend --author="First Last <first.last@email.com>"
>>>
>>> To apply it.
>>>
>> Sorry Lee, I will modify it. Is the order important? or only your
>> name is need here?
>>>> drivers/mfd/rtl8411.c: In function 'rtl8411_fetch_vendor_settings':
>>>> drivers/mfd/rtl8411.c:58:7: warning: 'reg1' is used uninitialized in this function [-Wuninitialized]
>>>> drivers/mfd/rtl8411.c: In function 'rtl8411b_fetch_vendor_settings':
>>>> drivers/mfd/rtl8411.c:79:7: warning: 'reg' is used uninitialized in this function [-Wuninitialized]
>>>> drivers/mfd/rtl8411.c: In function 'rtl8411_fetch_vendor_settings':
>>>> drivers/mfd/rtl8411.c:69:26: warning: 'reg3' may be used uninitialized in this function [-Wuninitialized]
>>>>
>>>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>>>> Tested-by: Micky Ching <micky_ching@realsil.com.cn>
>>> I know this is a bit of a strange situation, but whenever you sent
>>> patches to the MLs, you need to sign it off yourself too.
> Actually, you don't need to worry now. I already applied the patch
> with the correct author applied with your Ack.
>
>> Did you mean I need write like this ?
>>
>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>> Tested-by: Micky Ching <micky_ching@realsil.com.cn>
>> Signed-off-by: Micky Ching <micky_ching@realsil.com.cn>
> As I say, this situation is a bit different, but in the normal case,
> yes. Then the Maintainer will probably remove some and chop them
> around a little to his/her liking.
>
> FYI: The top SoB is usually the author, the subsequent ones are
> usually gatekeepers of upstream repos. Anyone can take a patch and
> submit it though, so in the case where the submitter isn't the author,
> the second SoB is usually the patch submitter.
>
Thank you for your patient.
--
Best Regards
Micky.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] mfd: rtsx: prevent 'used uninitialised' warnings
2013-12-03 10:54 ` micky
2013-12-03 11:01 ` Lee Jones
@ 2013-12-03 11:04 ` Dan Carpenter
1 sibling, 0 replies; 15+ messages in thread
From: Dan Carpenter @ 2013-12-03 11:04 UTC (permalink / raw)
To: micky; +Cc: Lee Jones, sameo, gregkh, linux-kernel, wei_wang, rogerable,
devel
Signed-off-by is intended to have a legal meaning that you didn't steal
someone's code. You can't sign off for other people.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 15+ messages in thread