| Message ID | 20210805174219.3000667-2-piyush.mehta@xilinx.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | gpio: modepin: Add driver support for modepin GPIO controller | expand |
On 8/5/21 7:42 PM, Piyush Mehta wrote: > Add Xilinx ZynqMP firmware MMIO APIs support to set and get PS_MODE > pins value and status. These APIs create an interface path between > mode pin controller driver and low-level API to access GPIO pins. > > Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com> > --- > Changes in v2: > - Added Xilinx ZynqMP firmware MMIO API support to set and get pin > value and status. > --- > drivers/firmware/xilinx/zynqmp.c | 46 ++++++++++++++++++++++++++++++++++++ > include/linux/firmware/xlnx-zynqmp.h | 14 +++++++++++ > 2 files changed, 60 insertions(+) > > diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c > index 15b13832..0234423 100644 > --- a/drivers/firmware/xilinx/zynqmp.c > +++ b/drivers/firmware/xilinx/zynqmp.c > @@ -28,6 +28,13 @@ > /* Max HashMap Order for PM API feature check (1<<7 = 128) */ > #define PM_API_FEATURE_CHECK_MAX_ORDER 7 > > +/* CRL registers and bitfields */ > +#define CRL_APB_BASE 0xFF5E0000U > +/* BOOT_PIN_CTRL- Used to control the mode pins after boot */ > +#define CRL_APB_BOOT_PIN_CTRL (CRL_APB_BASE + (0x250U)) > +/* BOOT_PIN_CTRL_MASK- out_val[11:8], out_en[3:0] */ > +#define CRL_APB_BOOTPIN_CTRL_MASK 0xF0FU > + > static bool feature_check_enabled; > static DEFINE_HASHTABLE(pm_api_features_map, PM_API_FEATURE_CHECK_MAX_ORDER); > > @@ -926,6 +933,45 @@ int zynqmp_pm_pinctrl_set_config(const u32 pin, const u32 param, > EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_set_config); > > /** > + * zynqmp_pm_bootmode_read() - PM Config API for read bootpin status > + * @ps_mode: Returned output value of ps_mode > + * > + * This API function is to be used for notify the power management controller > + * to read bootpin status. > + * > + * Return: status, either success or error+reason > + */ > +unsigned int zynqmp_pm_bootmode_read(u32 *ps_mode) > +{ > + unsigned int ret; > + u32 ret_payload[PAYLOAD_ARG_CNT]; > + > + ret = zynqmp_pm_invoke_fn(PM_MMIO_READ, CRL_APB_BOOT_PIN_CTRL, 0, > + 0, 0, ret_payload); > + > + *ps_mode = ret_payload[1]; > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(zynqmp_pm_bootmode_read); > + > +/** > + * zynqmp_pm_bootmode_write() - PM Config API for Configure bootpin > + * @ps_mode: Value to be written to the bootpin ctrl register > + * > + * This API function is to be used for notify the power management controller > + * to configure bootpin. > + * > + * Return: Returns status, either success or error+reason > + */ > +int zynqmp_pm_bootmode_write(u32 ps_mode) > +{ > + return zynqmp_pm_invoke_fn(PM_MMIO_WRITE, CRL_APB_BOOT_PIN_CTRL, > + CRL_APB_BOOTPIN_CTRL_MASK, ps_mode, 0, NULL); > +} > +EXPORT_SYMBOL_GPL(zynqmp_pm_bootmode_write); > + > +/** > * zynqmp_pm_init_finalize() - PM call to inform firmware that the caller > * master has initialized its own power management > * > diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h > index 9d1a5c1..dc6f39f 100644 > --- a/include/linux/firmware/xlnx-zynqmp.h > +++ b/include/linux/firmware/xlnx-zynqmp.h > @@ -68,6 +68,8 @@ enum pm_api_id { > PM_SET_REQUIREMENT = 15, > PM_RESET_ASSERT = 17, > PM_RESET_GET_STATUS = 18, > + PM_MMIO_WRITE = 19, > + PM_MMIO_READ = 20, > PM_PM_INIT_FINALIZE = 21, > PM_FPGA_LOAD = 22, > PM_FPGA_GET_STATUS = 23, > @@ -386,6 +388,8 @@ int zynqmp_pm_sd_dll_reset(u32 node_id, u32 type); > int zynqmp_pm_reset_assert(const enum zynqmp_pm_reset reset, > const enum zynqmp_pm_reset_action assert_flag); > int zynqmp_pm_reset_get_status(const enum zynqmp_pm_reset reset, u32 *status); > +unsigned int zynqmp_pm_bootmode_read(u32 *ps_mode); > +int zynqmp_pm_bootmode_write(u32 ps_mode); > int zynqmp_pm_init_finalize(void); > int zynqmp_pm_set_suspend_mode(u32 mode); > int zynqmp_pm_request_node(const u32 node, const u32 capabilities, > @@ -515,6 +519,16 @@ static inline int zynqmp_pm_reset_get_status(const enum zynqmp_pm_reset reset, > return -ENODEV; > } > > +static inline unsigned int zynqmp_pm_bootmode_read(u32 *ps_mode) > +{ > + return -ENODEV; > +} > + > +static inline int zynqmp_pm_bootmode_write(u32 ps_mode) > +{ > + return -ENODEV; > +} > + > static inline int zynqmp_pm_init_finalize(void) > { > return -ENODEV; > Acked-by: Michal Simek <michal.simek@xilinx.com> Thanks, Michal
On Thu, Aug 5, 2021 at 7:42 PM Piyush Mehta <piyush.mehta@xilinx.com> wrote: > Add Xilinx ZynqMP firmware MMIO APIs support to set and get PS_MODE > pins value and status. These APIs create an interface path between > mode pin controller driver and low-level API to access GPIO pins. > > Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com> > --- > Changes in v2: > - Added Xilinx ZynqMP firmware MMIO API support to set and get pin > value and status. I doubt this is "GPIO". General Purpose? I think not. It seems to be about boot mode. If you need a userspace ABI, then add sysfs files to this firmware driver instead of bridging it to the GPIO subsystem. However I can be argued down from usecases etc that it is used as GPIO but I need to push back on this. Yours, Linus Walleij
On Wed, Aug 11, 2021 at 3:08 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > On Thu, Aug 5, 2021 at 7:42 PM Piyush Mehta <piyush.mehta@xilinx.com> wrote: > > > Add Xilinx ZynqMP firmware MMIO APIs support to set and get PS_MODE > > pins value and status. These APIs create an interface path between > > mode pin controller driver and low-level API to access GPIO pins. > > > > Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com> > > --- > > Changes in v2: > > - Added Xilinx ZynqMP firmware MMIO API support to set and get pin > > value and status. > > I doubt this is "GPIO". > General Purpose? I think not. It seems to be about boot mode. Agreed. > If you need a userspace ABI, then add sysfs files to this firmware > driver instead of bridging it to the GPIO subsystem. I don't really want custom user interfaces in firmware drivers either. What is the high-level description of the 'PS_MODE' here? Is this perhaps something we already have a user interface for? Arnd
Hi Arnd, On 8/11/21 3:46 PM, Arnd Bergmann wrote: > On Wed, Aug 11, 2021 at 3:08 PM Linus Walleij <linus.walleij@linaro.org> wrote: >> >> On Thu, Aug 5, 2021 at 7:42 PM Piyush Mehta <piyush.mehta@xilinx.com> wrote: >> >>> Add Xilinx ZynqMP firmware MMIO APIs support to set and get PS_MODE >>> pins value and status. These APIs create an interface path between >>> mode pin controller driver and low-level API to access GPIO pins. >>> >>> Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com> >>> --- >>> Changes in v2: >>> - Added Xilinx ZynqMP firmware MMIO API support to set and get pin >>> value and status. >> >> I doubt this is "GPIO". >> General Purpose? I think not. It seems to be about boot mode. > > Agreed. here is register description. https://www.xilinx.com/html_docs/registers/ug1087/crl_apb___boot_pin_ctrl.html# > >> If you need a userspace ABI, then add sysfs files to this firmware >> driver instead of bridging it to the GPIO subsystem. > > I don't really want custom user interfaces in firmware drivers either. > > What is the high-level description of the 'PS_MODE' here? Is > this perhaps something we already have a user interface for? The reason why this can't be mapped as memory mapped device is that it is in IP which has be secure. That's why routing is done via firmware driver. Based on https://www.xilinx.com/support/documentation/user_guides/ug1085-zynq-ultrascale-trm.pdf page 46 PS_MODE Input/Output Dedicated 4-bit boot mode pins sampled on POR deassertion It means ROM just capture boot mode at start that's why they have special meaning and after it is free to use for whatever purpose you want which seems to pretty much as generic purpose I/O. I wrote more comments in reply to Linus already. Thanks, Michal
On Thu, Aug 5, 2021 at 7:42 PM Piyush Mehta <piyush.mehta@xilinx.com> wrote: > Add Xilinx ZynqMP firmware MMIO APIs support to set and get PS_MODE > pins value and status. These APIs create an interface path between > mode pin controller driver and low-level API to access GPIO pins. > > Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com> > --- > Changes in v2: After Michals description of how this is controlling USB PHY and misc resets I'm OK with the concept. Acked-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c index 15b13832..0234423 100644 --- a/drivers/firmware/xilinx/zynqmp.c +++ b/drivers/firmware/xilinx/zynqmp.c @@ -28,6 +28,13 @@ /* Max HashMap Order for PM API feature check (1<<7 = 128) */ #define PM_API_FEATURE_CHECK_MAX_ORDER 7 +/* CRL registers and bitfields */ +#define CRL_APB_BASE 0xFF5E0000U +/* BOOT_PIN_CTRL- Used to control the mode pins after boot */ +#define CRL_APB_BOOT_PIN_CTRL (CRL_APB_BASE + (0x250U)) +/* BOOT_PIN_CTRL_MASK- out_val[11:8], out_en[3:0] */ +#define CRL_APB_BOOTPIN_CTRL_MASK 0xF0FU + static bool feature_check_enabled; static DEFINE_HASHTABLE(pm_api_features_map, PM_API_FEATURE_CHECK_MAX_ORDER); @@ -926,6 +933,45 @@ int zynqmp_pm_pinctrl_set_config(const u32 pin, const u32 param, EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_set_config); /** + * zynqmp_pm_bootmode_read() - PM Config API for read bootpin status + * @ps_mode: Returned output value of ps_mode + * + * This API function is to be used for notify the power management controller + * to read bootpin status. + * + * Return: status, either success or error+reason + */ +unsigned int zynqmp_pm_bootmode_read(u32 *ps_mode) +{ + unsigned int ret; + u32 ret_payload[PAYLOAD_ARG_CNT]; + + ret = zynqmp_pm_invoke_fn(PM_MMIO_READ, CRL_APB_BOOT_PIN_CTRL, 0, + 0, 0, ret_payload); + + *ps_mode = ret_payload[1]; + + return ret; +} +EXPORT_SYMBOL_GPL(zynqmp_pm_bootmode_read); + +/** + * zynqmp_pm_bootmode_write() - PM Config API for Configure bootpin + * @ps_mode: Value to be written to the bootpin ctrl register + * + * This API function is to be used for notify the power management controller + * to configure bootpin. + * + * Return: Returns status, either success or error+reason + */ +int zynqmp_pm_bootmode_write(u32 ps_mode) +{ + return zynqmp_pm_invoke_fn(PM_MMIO_WRITE, CRL_APB_BOOT_PIN_CTRL, + CRL_APB_BOOTPIN_CTRL_MASK, ps_mode, 0, NULL); +} +EXPORT_SYMBOL_GPL(zynqmp_pm_bootmode_write); + +/** * zynqmp_pm_init_finalize() - PM call to inform firmware that the caller * master has initialized its own power management * diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h index 9d1a5c1..dc6f39f 100644 --- a/include/linux/firmware/xlnx-zynqmp.h +++ b/include/linux/firmware/xlnx-zynqmp.h @@ -68,6 +68,8 @@ enum pm_api_id { PM_SET_REQUIREMENT = 15, PM_RESET_ASSERT = 17, PM_RESET_GET_STATUS = 18, + PM_MMIO_WRITE = 19, + PM_MMIO_READ = 20, PM_PM_INIT_FINALIZE = 21, PM_FPGA_LOAD = 22, PM_FPGA_GET_STATUS = 23, @@ -386,6 +388,8 @@ int zynqmp_pm_sd_dll_reset(u32 node_id, u32 type); int zynqmp_pm_reset_assert(const enum zynqmp_pm_reset reset, const enum zynqmp_pm_reset_action assert_flag); int zynqmp_pm_reset_get_status(const enum zynqmp_pm_reset reset, u32 *status); +unsigned int zynqmp_pm_bootmode_read(u32 *ps_mode); +int zynqmp_pm_bootmode_write(u32 ps_mode); int zynqmp_pm_init_finalize(void); int zynqmp_pm_set_suspend_mode(u32 mode); int zynqmp_pm_request_node(const u32 node, const u32 capabilities, @@ -515,6 +519,16 @@ static inline int zynqmp_pm_reset_get_status(const enum zynqmp_pm_reset reset, return -ENODEV; } +static inline unsigned int zynqmp_pm_bootmode_read(u32 *ps_mode) +{ + return -ENODEV; +} + +static inline int zynqmp_pm_bootmode_write(u32 ps_mode) +{ + return -ENODEV; +} + static inline int zynqmp_pm_init_finalize(void) { return -ENODEV;
Add Xilinx ZynqMP firmware MMIO APIs support to set and get PS_MODE pins value and status. These APIs create an interface path between mode pin controller driver and low-level API to access GPIO pins. Signed-off-by: Piyush Mehta <piyush.mehta@xilinx.com> --- Changes in v2: - Added Xilinx ZynqMP firmware MMIO API support to set and get pin value and status. --- drivers/firmware/xilinx/zynqmp.c | 46 ++++++++++++++++++++++++++++++++++++ include/linux/firmware/xlnx-zynqmp.h | 14 +++++++++++ 2 files changed, 60 insertions(+)