| Message ID | 20180304211543.16824-1-ezequiel@vanguardiasur.com.ar |
|---|---|
| State | New |
| Headers | show |
| Series | pinctrl: ingenic: Fix MMC pins for 4770/4780 SoCs | expand |
Hi, Le 2018-03-04 22:15, Ezequiel Garcia a écrit : > This commit fixes the values for the pins and functions > of mmc0 and mmc1, for JZ4770 and JZ4780 SoCs. > > The bug was found on a Ci20 board, so changes are partially > tested on this board, in addition to careful verification > with the programming manual. > > Cc: Paul Cercueil <paul@crapouillou.net> > Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> The current values were obviously tested and do work with the CI20 (jz4780) and the GCW0 (which has a jz4770). Looking at the JZ4780 PM pdf, I don't think any of your values are correct. For instance, you set: static int jz4770_mmc0_1bit_a_pins[] = { 0x18, 0x19, 0x20, }; This means pins GPA24, GPA25 and GPB0. According to the manual, GPA24 is for MMC0 reset, but the other two are not in any way related to the MMC hardware. Regards, -Paul > --- > drivers/pinctrl/pinctrl-ingenic.c | 32 > +++++++++++++++++--------------- > 1 file changed, 17 insertions(+), 15 deletions(-) > > diff --git a/drivers/pinctrl/pinctrl-ingenic.c > b/drivers/pinctrl/pinctrl-ingenic.c > index 372ddf386bdb..3c05ed88acbd 100644 > --- a/drivers/pinctrl/pinctrl-ingenic.c > +++ b/drivers/pinctrl/pinctrl-ingenic.c > @@ -222,15 +222,17 @@ static int jz4770_uart2_hwflow_pins[] = { 0x65, > 0x64, }; > static int jz4770_uart3_data_pins[] = { 0x6c, 0x85, }; > static int jz4770_uart3_hwflow_pins[] = { 0x88, 0x89, }; > static int jz4770_uart4_data_pins[] = { 0x54, 0x4a, }; > -static int jz4770_mmc0_8bit_a_pins[] = { 0x04, 0x05, 0x06, 0x07, 0x18, > }; > -static int jz4770_mmc0_4bit_a_pins[] = { 0x15, 0x16, 0x17, }; > -static int jz4770_mmc0_1bit_a_pins[] = { 0x12, 0x13, 0x14, }; > -static int jz4770_mmc0_4bit_e_pins[] = { 0x95, 0x96, 0x97, }; > -static int jz4770_mmc0_1bit_e_pins[] = { 0x9c, 0x9d, 0x94, }; > -static int jz4770_mmc1_4bit_d_pins[] = { 0x75, 0x76, 0x77, }; > -static int jz4770_mmc1_1bit_d_pins[] = { 0x78, 0x79, 0x74, }; > -static int jz4770_mmc1_4bit_e_pins[] = { 0x95, 0x96, 0x97, }; > -static int jz4770_mmc1_1bit_e_pins[] = { 0x9c, 0x9d, 0x94, }; > +static int jz4770_mmc0_8bit_a_pins[] = { 0x04, 0x05, 0x06, 0x07, 0x18, > 0x19, > + 0x20, 0x21, 0x22, 0x23, }; > +static int jz4770_mmc0_4bit_a_pins[] = { 0x18, 0x19, 0x20, 0x21, 0x22, > 0x23 }; > +static int jz4770_mmc0_1bit_a_pins[] = { 0x18, 0x19, 0x20, }; > +static int jz4770_mmc0_4bit_e_pins[] = { 0x94, 0x95, 0x96, 0x97, 0x9c, > 0x9d }; > +static int jz4770_mmc0_1bit_e_pins[] = { 0x94, 0x9c, 0x9d, }; > + > +static int jz4770_mmc1_4bit_d_pins[] = { 0x74, 0x75, 0x76, 0x77, 0x78, > 0x79 }; > +static int jz4770_mmc1_1bit_d_pins[] = { 0x74, 0x78, 0x79 }; > +static int jz4770_mmc1_4bit_e_pins[] = { 0x94, 0x95, 0x96, 0x97, 0x9c, > 0x9d }; > +static int jz4770_mmc1_1bit_e_pins[] = { 0x94, 0x9c, 0x9d, }; > static int jz4770_nemc_data_pins[] = { > 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, > }; > @@ -277,14 +279,14 @@ static int jz4770_uart2_hwflow_funcs[] = { 1, 1, > }; > static int jz4770_uart3_data_funcs[] = { 0, 1, }; > static int jz4770_uart3_hwflow_funcs[] = { 0, 0, }; > static int jz4770_uart4_data_funcs[] = { 2, 2, }; > -static int jz4770_mmc0_8bit_a_funcs[] = { 1, 1, 1, 1, 1, }; > -static int jz4770_mmc0_4bit_a_funcs[] = { 1, 1, 1, }; > -static int jz4770_mmc0_1bit_a_funcs[] = { 1, 1, 0, }; > -static int jz4770_mmc0_4bit_e_funcs[] = { 0, 0, 0, }; > +static int jz4770_mmc0_8bit_a_funcs[] = { 1, 1, 1, 1, 1, 1, 1, 1, 1, > 1, }; > +static int jz4770_mmc0_4bit_a_funcs[] = { 1, 1, 1, 1, 1, 1, }; > +static int jz4770_mmc0_1bit_a_funcs[] = { 1, 1, 1, }; > +static int jz4770_mmc0_4bit_e_funcs[] = { 0, 0, 0, 0, 0, 0, }; > static int jz4770_mmc0_1bit_e_funcs[] = { 0, 0, 0, }; > -static int jz4770_mmc1_4bit_d_funcs[] = { 0, 0, 0, }; > +static int jz4770_mmc1_4bit_d_funcs[] = { 0, 0, 0, 0, 0, 0, }; > static int jz4770_mmc1_1bit_d_funcs[] = { 0, 0, 0, }; > -static int jz4770_mmc1_4bit_e_funcs[] = { 1, 1, 1, }; > +static int jz4770_mmc1_4bit_e_funcs[] = { 1, 1, 1, 1, 1, 1, }; > static int jz4770_mmc1_1bit_e_funcs[] = { 1, 1, 1, }; > static int jz4770_nemc_data_funcs[] = { 0, 0, 0, 0, 0, 0, 0, 0, }; > static int jz4770_nemc_cle_ale_funcs[] = { 0, 0, }; -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 4 March 2018 at 21:01, Paul Cercueil <paul@crapouillou.net> wrote: > Hi, > > Le 2018-03-04 22:15, Ezequiel Garcia a écrit : >> >> This commit fixes the values for the pins and functions >> of mmc0 and mmc1, for JZ4770 and JZ4780 SoCs. >> >> The bug was found on a Ci20 board, so changes are partially >> tested on this board, in addition to careful verification >> with the programming manual. >> >> Cc: Paul Cercueil <paul@crapouillou.net> >> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > > > The current values were obviously tested and do work with the CI20 (jz4780) > and the GCW0 (which has a jz4770). > Of course, maybe it's something else. I've pushed a branch with MMC supported forwared-ported to current mainline, in case you feel like giving a try. It wasn't working for me, and this commit fixed it. http://git.infradead.org/users/ezequielg/linux/shortlog/refs/heads/ci20-mmc-for-4.15 > Looking at the JZ4780 PM pdf, I don't think any of your values are correct. > > For instance, you set: > static int jz4770_mmc0_1bit_a_pins[] = { 0x18, 0x19, 0x20, }; > > This means pins GPA24, GPA25 and GPB0. Doh, of course. My changes to the PA muxing are just bullshit. > According to the manual, GPA24 is for > MMC0 reset, but the other two are not in any way related to the MMC > hardware. > Let's look a those that were actually tested. For instance: >> -static int jz4770_mmc0_4bit_e_pins[] = { 0x95, 0x96, 0x97, }; >> +static int jz4770_mmc0_4bit_e_pins[] = { 0x94, 0x95, 0x96, 0x97, 0x9c, >> 0x9d }; IIUC, the driver is muxing pins PE21, PE22 and PE23, which belong to msc0_d1, msc0_d2 and msc0_d3. According to my spec, it should mux PE20, PE21, PE22, PE23, PE28 and PE29 (d0, d1, d2, d3, clk, cmd). Which is what my patch is trying to do, if I did the math right. I'm confused about the fact that a 4-bit mode muxes just three pins, instead of six. So what am I missing here? Thanks,
Le lun. 5 mars 2018 à 3:29, Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> a écrit : > On 4 March 2018 at 21:01, Paul Cercueil <paul@crapouillou.net> wrote: >> Hi, >> >> Le 2018-03-04 22:15, Ezequiel Garcia a écrit : >>> >>> This commit fixes the values for the pins and functions >>> of mmc0 and mmc1, for JZ4770 and JZ4780 SoCs. >>> >>> The bug was found on a Ci20 board, so changes are partially >>> tested on this board, in addition to careful verification >>> with the programming manual. >>> >>> Cc: Paul Cercueil <paul@crapouillou.net> >>> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> >> >> >> The current values were obviously tested and do work with the CI20 >> (jz4780) >> and the GCW0 (which has a jz4770). >> > > Of course, maybe it's something else. > > I've pushed a branch with MMC supported forwared-ported > to current mainline, in case you feel like giving a try. > It wasn't working for me, and this commit fixed it. > > http://git.infradead.org/users/ezequielg/linux/shortlog/refs/heads/ci20-mmc-for-4.15 > >> Looking at the JZ4780 PM pdf, I don't think any of your values are >> correct. >> >> For instance, you set: >> static int jz4770_mmc0_1bit_a_pins[] = { 0x18, 0x19, 0x20, }; >> >> This means pins GPA24, GPA25 and GPB0. > > Doh, of course. My changes to the PA muxing are just bullshit. > >> According to the manual, GPA24 is for >> MMC0 reset, but the other two are not in any way related to the MMC >> hardware. >> > > Let's look a those that were actually tested. For instance: > >>> -static int jz4770_mmc0_4bit_e_pins[] = { 0x95, 0x96, 0x97, }; >>> +static int jz4770_mmc0_4bit_e_pins[] = { 0x94, 0x95, 0x96, 0x97, >>> 0x9c, >>> 0x9d }; > > IIUC, the driver is muxing pins PE21, PE22 and PE23, which > belong to msc0_d1, msc0_d2 and msc0_d3. > > According to my spec, it should mux PE20, PE21, PE22, PE23, > PE28 and PE29 (d0, d1, d2, d3, clk, cmd). Which is what my > patch is trying to do, if I did the math right. > > I'm confused about the fact that a 4-bit mode muxes > just three pins, instead of six. So what am I missing here? The jz4770_mmc0_1bit_e_pins[] contains the pins required for doing *at least* 1-bit I/O. The jz4770_mmc0_4bit_e_pins[] contains the *additional* pins required to do 4-bit I/O. In your devicetree, if you plan to use 4-bit I/O, you must enable the two groups: pins_mmc0_4bit: mmc0-4bit { function = "mmc0"; groups = "mmc0-1bit-e", "mmc0-4bit-e"; bias-disable; }; > Thanks, > -- > Ezequiel García, VanguardiaSur > www.vanguardiasur.com.ar -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 5 March 2018 at 12:56, Paul Cercueil <paul@crapouillou.net> wrote: > > > Le lun. 5 mars 2018 à 3:29, Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > a écrit : >> >> On 4 March 2018 at 21:01, Paul Cercueil <paul@crapouillou.net> wrote: >>> >>> Hi, >>> >>> Le 2018-03-04 22:15, Ezequiel Garcia a écrit : >>>> >>>> >>>> This commit fixes the values for the pins and functions >>>> of mmc0 and mmc1, for JZ4770 and JZ4780 SoCs. >>>> >>>> The bug was found on a Ci20 board, so changes are partially >>>> tested on this board, in addition to careful verification >>>> with the programming manual. >>>> >>>> Cc: Paul Cercueil <paul@crapouillou.net> >>>> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> >>> >>> >>> >>> The current values were obviously tested and do work with the CI20 >>> (jz4780) >>> and the GCW0 (which has a jz4770). >>> >> >> Of course, maybe it's something else. >> >> I've pushed a branch with MMC supported forwared-ported >> to current mainline, in case you feel like giving a try. >> It wasn't working for me, and this commit fixed it. >> >> >> http://git.infradead.org/users/ezequielg/linux/shortlog/refs/heads/ci20-mmc-for-4.15 >> >>> Looking at the JZ4780 PM pdf, I don't think any of your values are >>> correct. >>> >>> For instance, you set: >>> static int jz4770_mmc0_1bit_a_pins[] = { 0x18, 0x19, 0x20, }; >>> >>> This means pins GPA24, GPA25 and GPB0. >> >> >> Doh, of course. My changes to the PA muxing are just bullshit. >> >>> According to the manual, GPA24 is for >>> MMC0 reset, but the other two are not in any way related to the MMC >>> hardware. >>> >> >> Let's look a those that were actually tested. For instance: >> >>>> -static int jz4770_mmc0_4bit_e_pins[] = { 0x95, 0x96, 0x97, }; >>>> +static int jz4770_mmc0_4bit_e_pins[] = { 0x94, 0x95, 0x96, 0x97, 0x9c, >>>> 0x9d }; >> >> >> IIUC, the driver is muxing pins PE21, PE22 and PE23, which >> belong to msc0_d1, msc0_d2 and msc0_d3. >> >> According to my spec, it should mux PE20, PE21, PE22, PE23, >> PE28 and PE29 (d0, d1, d2, d3, clk, cmd). Which is what my >> patch is trying to do, if I did the math right. >> >> I'm confused about the fact that a 4-bit mode muxes >> just three pins, instead of six. So what am I missing here? > > > The jz4770_mmc0_1bit_e_pins[] contains the pins required for doing *at > least* > 1-bit I/O. The jz4770_mmc0_4bit_e_pins[] contains the *additional* pins > required to do 4-bit I/O. *facepalm* > In your devicetree, if you plan to use 4-bit I/O, you must enable the two > groups: > > pins_mmc0_4bit: mmc0-4bit { > function = "mmc0"; > groups = "mmc0-1bit-e", "mmc0-4bit-e"; > bias-disable; > }; > > That solves the riddle then. I'll add this to the bindings documentation for MMC. This means this patch is just BS. Thanks the for help, Paul!
diff --git a/drivers/pinctrl/pinctrl-ingenic.c b/drivers/pinctrl/pinctrl-ingenic.c index 372ddf386bdb..3c05ed88acbd 100644 --- a/drivers/pinctrl/pinctrl-ingenic.c +++ b/drivers/pinctrl/pinctrl-ingenic.c @@ -222,15 +222,17 @@ static int jz4770_uart2_hwflow_pins[] = { 0x65, 0x64, }; static int jz4770_uart3_data_pins[] = { 0x6c, 0x85, }; static int jz4770_uart3_hwflow_pins[] = { 0x88, 0x89, }; static int jz4770_uart4_data_pins[] = { 0x54, 0x4a, }; -static int jz4770_mmc0_8bit_a_pins[] = { 0x04, 0x05, 0x06, 0x07, 0x18, }; -static int jz4770_mmc0_4bit_a_pins[] = { 0x15, 0x16, 0x17, }; -static int jz4770_mmc0_1bit_a_pins[] = { 0x12, 0x13, 0x14, }; -static int jz4770_mmc0_4bit_e_pins[] = { 0x95, 0x96, 0x97, }; -static int jz4770_mmc0_1bit_e_pins[] = { 0x9c, 0x9d, 0x94, }; -static int jz4770_mmc1_4bit_d_pins[] = { 0x75, 0x76, 0x77, }; -static int jz4770_mmc1_1bit_d_pins[] = { 0x78, 0x79, 0x74, }; -static int jz4770_mmc1_4bit_e_pins[] = { 0x95, 0x96, 0x97, }; -static int jz4770_mmc1_1bit_e_pins[] = { 0x9c, 0x9d, 0x94, }; +static int jz4770_mmc0_8bit_a_pins[] = { 0x04, 0x05, 0x06, 0x07, 0x18, 0x19, + 0x20, 0x21, 0x22, 0x23, }; +static int jz4770_mmc0_4bit_a_pins[] = { 0x18, 0x19, 0x20, 0x21, 0x22, 0x23 }; +static int jz4770_mmc0_1bit_a_pins[] = { 0x18, 0x19, 0x20, }; +static int jz4770_mmc0_4bit_e_pins[] = { 0x94, 0x95, 0x96, 0x97, 0x9c, 0x9d }; +static int jz4770_mmc0_1bit_e_pins[] = { 0x94, 0x9c, 0x9d, }; + +static int jz4770_mmc1_4bit_d_pins[] = { 0x74, 0x75, 0x76, 0x77, 0x78, 0x79 }; +static int jz4770_mmc1_1bit_d_pins[] = { 0x74, 0x78, 0x79 }; +static int jz4770_mmc1_4bit_e_pins[] = { 0x94, 0x95, 0x96, 0x97, 0x9c, 0x9d }; +static int jz4770_mmc1_1bit_e_pins[] = { 0x94, 0x9c, 0x9d, }; static int jz4770_nemc_data_pins[] = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, }; @@ -277,14 +279,14 @@ static int jz4770_uart2_hwflow_funcs[] = { 1, 1, }; static int jz4770_uart3_data_funcs[] = { 0, 1, }; static int jz4770_uart3_hwflow_funcs[] = { 0, 0, }; static int jz4770_uart4_data_funcs[] = { 2, 2, }; -static int jz4770_mmc0_8bit_a_funcs[] = { 1, 1, 1, 1, 1, }; -static int jz4770_mmc0_4bit_a_funcs[] = { 1, 1, 1, }; -static int jz4770_mmc0_1bit_a_funcs[] = { 1, 1, 0, }; -static int jz4770_mmc0_4bit_e_funcs[] = { 0, 0, 0, }; +static int jz4770_mmc0_8bit_a_funcs[] = { 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, }; +static int jz4770_mmc0_4bit_a_funcs[] = { 1, 1, 1, 1, 1, 1, }; +static int jz4770_mmc0_1bit_a_funcs[] = { 1, 1, 1, }; +static int jz4770_mmc0_4bit_e_funcs[] = { 0, 0, 0, 0, 0, 0, }; static int jz4770_mmc0_1bit_e_funcs[] = { 0, 0, 0, }; -static int jz4770_mmc1_4bit_d_funcs[] = { 0, 0, 0, }; +static int jz4770_mmc1_4bit_d_funcs[] = { 0, 0, 0, 0, 0, 0, }; static int jz4770_mmc1_1bit_d_funcs[] = { 0, 0, 0, }; -static int jz4770_mmc1_4bit_e_funcs[] = { 1, 1, 1, }; +static int jz4770_mmc1_4bit_e_funcs[] = { 1, 1, 1, 1, 1, 1, }; static int jz4770_mmc1_1bit_e_funcs[] = { 1, 1, 1, }; static int jz4770_nemc_data_funcs[] = { 0, 0, 0, 0, 0, 0, 0, 0, }; static int jz4770_nemc_cle_ale_funcs[] = { 0, 0, };
This commit fixes the values for the pins and functions of mmc0 and mmc1, for JZ4770 and JZ4780 SoCs. The bug was found on a Ci20 board, so changes are partially tested on this board, in addition to careful verification with the programming manual. Cc: Paul Cercueil <paul@crapouillou.net> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> --- drivers/pinctrl/pinctrl-ingenic.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-)