On 17/06/2024 12:52, Geert Uytterhoeven wrote: > Hi Paul, > > On Tue, Jun 11, 2024 at 1:32 PM Paul Barker > <paul.barker.ct@xxxxxxxxxxxxxx> wrote: >> We currently support OEN read/write for the RZ/G3S SoC but not the >> RZ/G2L SoC family (consisting of RZ/G2L, RZ/G2LC, RZ/G2UL, RZ/V2L & >> RZ/Five). The appropriate functions are renamed to clarify this. >> >> We should also only set the oen_read and oen_write function pointers for >> the devices which support these operations. This requires us to check >> that these function pointers are valid before calling them. >> >> Signed-off-by: Paul Barker <paul.barker.ct@xxxxxxxxxxxxxx> >> --- >> Changes v1->v2: >> * New patch to clarify function names. > > Thanks for your patch! > >> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c >> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c > >> @@ -1016,31 +1016,31 @@ static u8 rzg2l_pin_to_oen_bit(u32 offset, u8 pin, u8 max_port) >> return pin; >> } >> >> -static u32 rzg2l_read_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin) >> +static u32 rzg3s_read_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin) > >> -static int rzg2l_write_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen) >> +static int rzg3s_write_oen(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin, u8 oen) > > As commit 7d566a4d270c52ff ("pinctrl: renesas: rzg2l: Add function > pointers for OEN register access") did not rename > rzg2l_{read,write}_oen() to rzg2l_oen_{read,write}(), to match the > .oen_{read,write}() callback names, this is a good opportunity to fix > that oversight. Ack. > > The v2h variants already match the callback names. > >> @@ -1215,6 +1215,8 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev, >> break; >> >> case PIN_CONFIG_OUTPUT_ENABLE: >> + if (!pctrl->data->oen_read) >> + return -EOPNOTSUPP; > > Perhaps the check for PIN_CFG_OEN in each of the .oen_read() > callbacks should be moved here? Ack. > >> arg = pctrl->data->oen_read(pctrl, cfg, _pin, bit); >> if (!arg) >> return -EINVAL; >> @@ -1354,6 +1356,8 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev, >> >> case PIN_CONFIG_OUTPUT_ENABLE: >> arg = pinconf_to_config_argument(_configs[i]); >> + if (!pctrl->data->oen_write) >> + return -EOPNOTSUPP; > > Likewise. Ack. I'll fix these in v3. Thanks, -- Paul Barker
Attachment:
OpenPGP_0x27F4B3459F002257.asc
Description: OpenPGP public key
Attachment:
OpenPGP_signature.asc
Description: OpenPGP digital signature