* [PATCH 0/3] firmware: xilinx: Add support to get last reset reason
@ 2020-09-10 0:20 Amit Sunil Dhamne
2020-09-10 0:20 ` [PATCH 1/3] firmware: xilinx: Add validation check for IOCTL Amit Sunil Dhamne
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Amit Sunil Dhamne @ 2020-09-10 0:20 UTC (permalink / raw)
To: ard.biesheuvel, mingo, gregkh, matt, sudeep.holla, hkallweit1,
keescook, dmitry.torokhov, michal.simek, rajanv, tejas.patel,
jollys
Cc: jolly.shah, rajan.vaja, Amit Sunil Dhamne, linux-kernel,
linux-arm-kernel
Add support to get last reset reason for Xilinx Versal platform.
Tejas Patel (3):
firmware: xilinx: Add validation check for IOCTL
firmware: xilinx: Add support for GET_LAST_RESET_REASON IOCTL
firmware: xilinx: Add sysfs to get last reset reason
.../ABI/stable/sysfs-driver-firmware-zynqmp | 13 ++
drivers/firmware/xilinx/zynqmp.c | 174 ++++++++++++++++++---
include/linux/firmware/xlnx-zynqmp.h | 18 +++
3 files changed, 183 insertions(+), 22 deletions(-)
--
2.7.4
This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/3] firmware: xilinx: Add validation check for IOCTL 2020-09-10 0:20 [PATCH 0/3] firmware: xilinx: Add support to get last reset reason Amit Sunil Dhamne @ 2020-09-10 0:20 ` Amit Sunil Dhamne 2020-09-16 11:33 ` Greg KH 2020-09-17 6:45 ` Punit Agrawal 2020-09-10 0:20 ` [PATCH 2/3] firmware: xilinx: Add support for GET_LAST_RESET_REASON IOCTL Amit Sunil Dhamne 2020-09-10 0:20 ` [PATCH 3/3] firmware: xilinx: Add sysfs to get last reset reason Amit Sunil Dhamne 2 siblings, 2 replies; 7+ messages in thread From: Amit Sunil Dhamne @ 2020-09-10 0:20 UTC (permalink / raw) To: ard.biesheuvel, mingo, gregkh, matt, sudeep.holla, hkallweit1, keescook, dmitry.torokhov, michal.simek, rajanv, tejas.patel, jollys Cc: jolly.shah, rajan.vaja, Amit Sunil Dhamne, linux-kernel, linux-arm-kernel From: Tejas Patel <tejas.patel@xilinx.com> Validate IOCTL ID for ZynqMP and Versal before calling zynqmp_pm_invoke_fn(). Signed-off-by: Tejas Patel <tejas.patel@xilinx.com> Signed-off-by: Amit Sunil Dhamne <amit.sunil.dhamne@xilinx.com> --- drivers/firmware/xilinx/zynqmp.c | 117 +++++++++++++++++++++++++++++++-------- 1 file changed, 95 insertions(+), 22 deletions(-) diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c index 8d1ff24..8fe0912 100644 --- a/drivers/firmware/xilinx/zynqmp.c +++ b/drivers/firmware/xilinx/zynqmp.c @@ -514,6 +514,89 @@ int zynqmp_pm_clock_getparent(u32 clock_id, u32 *parent_id) EXPORT_SYMBOL_GPL(zynqmp_pm_clock_getparent); /** + * versal_is_valid_ioctl() - Check whether IOCTL ID is valid or not for versal + * @ioctl_id: IOCTL ID + * + * Return: 1 if IOCTL is valid else 0 + */ +static inline int versal_is_valid_ioctl(u32 ioctl_id) +{ + switch (ioctl_id) { + case IOCTL_SD_DLL_RESET: + case IOCTL_SET_SD_TAPDELAY: + case IOCTL_SET_PLL_FRAC_MODE: + case IOCTL_GET_PLL_FRAC_MODE: + case IOCTL_SET_PLL_FRAC_DATA: + case IOCTL_GET_PLL_FRAC_DATA: + case IOCTL_WRITE_GGS: + case IOCTL_READ_GGS: + case IOCTL_WRITE_PGGS: + case IOCTL_READ_PGGS: + case IOCTL_SET_BOOT_HEALTH_STATUS: + return 1; + default: + return 0; + } +} + +/** + * zynqmp_is_valid_ioctl() - Check whether IOCTL ID is valid or not + * @ioctl_id: IOCTL ID + * + * Return: 1 if IOCTL is valid else 0 + */ +static inline int zynqmp_is_valid_ioctl(u32 ioctl_id) +{ + switch (ioctl_id) { + case IOCTL_SD_DLL_RESET: + case IOCTL_SET_SD_TAPDELAY: + case IOCTL_SET_PLL_FRAC_MODE: + case IOCTL_GET_PLL_FRAC_MODE: + case IOCTL_SET_PLL_FRAC_DATA: + case IOCTL_GET_PLL_FRAC_DATA: + case IOCTL_WRITE_GGS: + case IOCTL_READ_GGS: + case IOCTL_WRITE_PGGS: + case IOCTL_READ_PGGS: + case IOCTL_SET_BOOT_HEALTH_STATUS: + return 1; + default: + return 0; + } +} + +/** + * zynqmp_pm_ioctl() - PM IOCTL API for device control and configs + * @node_id: Node ID of the device + * @ioctl_id: ID of the requested IOCTL + * @arg1: Argument 1 to requested IOCTL call + * @arg2: Argument 2 to requested IOCTL call + * @out: Returned output value + * + * This function calls IOCTL to firmware for device control and configuration. + * + * Return: Returns status, either success or error+reason + */ +static int zynqmp_pm_ioctl(u32 node_id, u32 ioctl_id, u32 arg1, u32 arg2, + u32 *out) +{ + struct device_node *np; + + np = of_find_compatible_node(NULL, NULL, "xlnx,versal"); + if (np) { + if (!versal_is_valid_ioctl(ioctl_id)) + return -EINVAL; + } else { + if (!zynqmp_is_valid_ioctl(ioctl_id)) + return -EINVAL; + } + of_node_put(np); + + return zynqmp_pm_invoke_fn(PM_IOCTL, node_id, ioctl_id, arg1, arg2, + out); +} + +/** * zynqmp_pm_set_pll_frac_mode() - PM API for set PLL mode * * @clk_id: PLL clock ID @@ -525,8 +608,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_clock_getparent); */ int zynqmp_pm_set_pll_frac_mode(u32 clk_id, u32 mode) { - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_SET_PLL_FRAC_MODE, - clk_id, mode, NULL); + return zynqmp_pm_ioctl(0, IOCTL_SET_PLL_FRAC_MODE, clk_id, mode, NULL); } EXPORT_SYMBOL_GPL(zynqmp_pm_set_pll_frac_mode); @@ -542,8 +624,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_set_pll_frac_mode); */ int zynqmp_pm_get_pll_frac_mode(u32 clk_id, u32 *mode) { - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_GET_PLL_FRAC_MODE, - clk_id, 0, mode); + return zynqmp_pm_ioctl(0, IOCTL_GET_PLL_FRAC_MODE, clk_id, 0, mode); } EXPORT_SYMBOL_GPL(zynqmp_pm_get_pll_frac_mode); @@ -560,8 +641,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_get_pll_frac_mode); */ int zynqmp_pm_set_pll_frac_data(u32 clk_id, u32 data) { - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_SET_PLL_FRAC_DATA, - clk_id, data, NULL); + return zynqmp_pm_ioctl(0, IOCTL_SET_PLL_FRAC_DATA, clk_id, data, NULL); } EXPORT_SYMBOL_GPL(zynqmp_pm_set_pll_frac_data); @@ -577,8 +657,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_set_pll_frac_data); */ int zynqmp_pm_get_pll_frac_data(u32 clk_id, u32 *data) { - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_GET_PLL_FRAC_DATA, - clk_id, 0, data); + return zynqmp_pm_ioctl(0, IOCTL_GET_PLL_FRAC_DATA, clk_id, 0, data); } EXPORT_SYMBOL_GPL(zynqmp_pm_get_pll_frac_data); @@ -595,8 +674,8 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_get_pll_frac_data); */ int zynqmp_pm_set_sd_tapdelay(u32 node_id, u32 type, u32 value) { - return zynqmp_pm_invoke_fn(PM_IOCTL, node_id, IOCTL_SET_SD_TAPDELAY, - type, value, NULL); + return zynqmp_pm_ioctl(node_id, IOCTL_SET_SD_TAPDELAY, type, value, + NULL); } EXPORT_SYMBOL_GPL(zynqmp_pm_set_sd_tapdelay); @@ -612,8 +691,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_set_sd_tapdelay); */ int zynqmp_pm_sd_dll_reset(u32 node_id, u32 type) { - return zynqmp_pm_invoke_fn(PM_IOCTL, node_id, IOCTL_SET_SD_TAPDELAY, - type, 0, NULL); + return zynqmp_pm_ioctl(node_id, IOCTL_SET_SD_TAPDELAY, type, 0, NULL); } EXPORT_SYMBOL_GPL(zynqmp_pm_sd_dll_reset); @@ -628,8 +706,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_sd_dll_reset); */ int zynqmp_pm_write_ggs(u32 index, u32 value) { - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_WRITE_GGS, - index, value, NULL); + return zynqmp_pm_ioctl(0, IOCTL_WRITE_GGS, index, value, NULL); } EXPORT_SYMBOL_GPL(zynqmp_pm_write_ggs); @@ -644,8 +721,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_write_ggs); */ int zynqmp_pm_read_ggs(u32 index, u32 *value) { - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_READ_GGS, - index, 0, value); + return zynqmp_pm_ioctl(0, IOCTL_READ_GGS, index, 0, value); } EXPORT_SYMBOL_GPL(zynqmp_pm_read_ggs); @@ -661,8 +737,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_read_ggs); */ int zynqmp_pm_write_pggs(u32 index, u32 value) { - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_WRITE_PGGS, index, value, - NULL); + return zynqmp_pm_ioctl(0, IOCTL_WRITE_PGGS, index, value, NULL); } EXPORT_SYMBOL_GPL(zynqmp_pm_write_pggs); @@ -678,8 +753,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_write_pggs); */ int zynqmp_pm_read_pggs(u32 index, u32 *value) { - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_READ_PGGS, index, 0, - value); + return zynqmp_pm_ioctl(0, IOCTL_READ_PGGS, index, 0, value); } EXPORT_SYMBOL_GPL(zynqmp_pm_read_pggs); @@ -694,8 +768,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_read_pggs); */ int zynqmp_pm_set_boot_health_status(u32 value) { - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_SET_BOOT_HEALTH_STATUS, - value, 0, NULL); + return zynqmp_pm_ioctl(0, IOCTL_SET_BOOT_HEALTH_STATUS, value, 0, NULL); } /** -- 2.7.4 This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] firmware: xilinx: Add validation check for IOCTL 2020-09-10 0:20 ` [PATCH 1/3] firmware: xilinx: Add validation check for IOCTL Amit Sunil Dhamne @ 2020-09-16 11:33 ` Greg KH 2020-09-16 16:53 ` Amit Sunil Dhamne 2020-09-17 6:45 ` Punit Agrawal 1 sibling, 1 reply; 7+ messages in thread From: Greg KH @ 2020-09-16 11:33 UTC (permalink / raw) To: Amit Sunil Dhamne Cc: tejas.patel, keescook, rajan.vaja, ard.biesheuvel, matt, dmitry.torokhov, michal.simek, linux-kernel, jollys, rajanv, sudeep.holla, jolly.shah, mingo, linux-arm-kernel, hkallweit1 On Wed, Sep 09, 2020 at 05:20:02PM -0700, Amit Sunil Dhamne wrote: > From: Tejas Patel <tejas.patel@xilinx.com> > > Validate IOCTL ID for ZynqMP and Versal before calling > zynqmp_pm_invoke_fn(). > > Signed-off-by: Tejas Patel <tejas.patel@xilinx.com> > Signed-off-by: Amit Sunil Dhamne <amit.sunil.dhamne@xilinx.com> > --- > drivers/firmware/xilinx/zynqmp.c | 117 +++++++++++++++++++++++++++++++-------- > 1 file changed, 95 insertions(+), 22 deletions(-) > > diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c > index 8d1ff24..8fe0912 100644 > --- a/drivers/firmware/xilinx/zynqmp.c > +++ b/drivers/firmware/xilinx/zynqmp.c > @@ -514,6 +514,89 @@ int zynqmp_pm_clock_getparent(u32 clock_id, u32 *parent_id) > EXPORT_SYMBOL_GPL(zynqmp_pm_clock_getparent); > > /** > + * versal_is_valid_ioctl() - Check whether IOCTL ID is valid or not for versal > + * @ioctl_id: IOCTL ID > + * > + * Return: 1 if IOCTL is valid else 0 > + */ > +static inline int versal_is_valid_ioctl(u32 ioctl_id) > +{ > + switch (ioctl_id) { > + case IOCTL_SD_DLL_RESET: > + case IOCTL_SET_SD_TAPDELAY: > + case IOCTL_SET_PLL_FRAC_MODE: > + case IOCTL_GET_PLL_FRAC_MODE: > + case IOCTL_SET_PLL_FRAC_DATA: > + case IOCTL_GET_PLL_FRAC_DATA: > + case IOCTL_WRITE_GGS: > + case IOCTL_READ_GGS: > + case IOCTL_WRITE_PGGS: > + case IOCTL_READ_PGGS: > + case IOCTL_SET_BOOT_HEALTH_STATUS: > + return 1; > + default: > + return 0; bool is nicer, right? > + } > +} > + > +/** > + * zynqmp_is_valid_ioctl() - Check whether IOCTL ID is valid or not > + * @ioctl_id: IOCTL ID > + * > + * Return: 1 if IOCTL is valid else 0 > + */ > +static inline int zynqmp_is_valid_ioctl(u32 ioctl_id) > +{ > + switch (ioctl_id) { > + case IOCTL_SD_DLL_RESET: > + case IOCTL_SET_SD_TAPDELAY: > + case IOCTL_SET_PLL_FRAC_MODE: > + case IOCTL_GET_PLL_FRAC_MODE: > + case IOCTL_SET_PLL_FRAC_DATA: > + case IOCTL_GET_PLL_FRAC_DATA: > + case IOCTL_WRITE_GGS: > + case IOCTL_READ_GGS: > + case IOCTL_WRITE_PGGS: > + case IOCTL_READ_PGGS: > + case IOCTL_SET_BOOT_HEALTH_STATUS: > + return 1; > + default: > + return 0; > + } > +} > + > +/** > + * zynqmp_pm_ioctl() - PM IOCTL API for device control and configs > + * @node_id: Node ID of the device > + * @ioctl_id: ID of the requested IOCTL > + * @arg1: Argument 1 to requested IOCTL call > + * @arg2: Argument 2 to requested IOCTL call > + * @out: Returned output value > + * > + * This function calls IOCTL to firmware for device control and configuration. > + * > + * Return: Returns status, either success or error+reason > + */ > +static int zynqmp_pm_ioctl(u32 node_id, u32 ioctl_id, u32 arg1, u32 arg2, > + u32 *out) > +{ > + struct device_node *np; > + > + np = of_find_compatible_node(NULL, NULL, "xlnx,versal"); > + if (np) { > + if (!versal_is_valid_ioctl(ioctl_id)) > + return -EINVAL; Wrong error value. > + } else { > + if (!zynqmp_is_valid_ioctl(ioctl_id)) > + return -EINVAL; Wrong error value. > + } > + of_node_put(np); > + > + return zynqmp_pm_invoke_fn(PM_IOCTL, node_id, ioctl_id, arg1, arg2, > + out); No other checking of ioctl commands and arguments? Brave... > +} > + > +/** > * zynqmp_pm_set_pll_frac_mode() - PM API for set PLL mode > * > * @clk_id: PLL clock ID > @@ -525,8 +608,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_clock_getparent); > */ > int zynqmp_pm_set_pll_frac_mode(u32 clk_id, u32 mode) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_SET_PLL_FRAC_MODE, > - clk_id, mode, NULL); > + return zynqmp_pm_ioctl(0, IOCTL_SET_PLL_FRAC_MODE, clk_id, mode, NULL); > } > EXPORT_SYMBOL_GPL(zynqmp_pm_set_pll_frac_mode); > > @@ -542,8 +624,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_set_pll_frac_mode); > */ > int zynqmp_pm_get_pll_frac_mode(u32 clk_id, u32 *mode) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_GET_PLL_FRAC_MODE, > - clk_id, 0, mode); > + return zynqmp_pm_ioctl(0, IOCTL_GET_PLL_FRAC_MODE, clk_id, 0, mode); > } > EXPORT_SYMBOL_GPL(zynqmp_pm_get_pll_frac_mode); > > @@ -560,8 +641,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_get_pll_frac_mode); > */ > int zynqmp_pm_set_pll_frac_data(u32 clk_id, u32 data) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_SET_PLL_FRAC_DATA, > - clk_id, data, NULL); > + return zynqmp_pm_ioctl(0, IOCTL_SET_PLL_FRAC_DATA, clk_id, data, NULL); > } > EXPORT_SYMBOL_GPL(zynqmp_pm_set_pll_frac_data); > > @@ -577,8 +657,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_set_pll_frac_data); > */ > int zynqmp_pm_get_pll_frac_data(u32 clk_id, u32 *data) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_GET_PLL_FRAC_DATA, > - clk_id, 0, data); > + return zynqmp_pm_ioctl(0, IOCTL_GET_PLL_FRAC_DATA, clk_id, 0, data); > } > EXPORT_SYMBOL_GPL(zynqmp_pm_get_pll_frac_data); > > @@ -595,8 +674,8 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_get_pll_frac_data); > */ > int zynqmp_pm_set_sd_tapdelay(u32 node_id, u32 type, u32 value) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, node_id, IOCTL_SET_SD_TAPDELAY, > - type, value, NULL); > + return zynqmp_pm_ioctl(node_id, IOCTL_SET_SD_TAPDELAY, type, value, > + NULL); > } > EXPORT_SYMBOL_GPL(zynqmp_pm_set_sd_tapdelay); > > @@ -612,8 +691,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_set_sd_tapdelay); > */ > int zynqmp_pm_sd_dll_reset(u32 node_id, u32 type) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, node_id, IOCTL_SET_SD_TAPDELAY, > - type, 0, NULL); > + return zynqmp_pm_ioctl(node_id, IOCTL_SET_SD_TAPDELAY, type, 0, NULL); > } > EXPORT_SYMBOL_GPL(zynqmp_pm_sd_dll_reset); > > @@ -628,8 +706,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_sd_dll_reset); > */ > int zynqmp_pm_write_ggs(u32 index, u32 value) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_WRITE_GGS, > - index, value, NULL); > + return zynqmp_pm_ioctl(0, IOCTL_WRITE_GGS, index, value, NULL); > } > EXPORT_SYMBOL_GPL(zynqmp_pm_write_ggs); > > @@ -644,8 +721,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_write_ggs); > */ > int zynqmp_pm_read_ggs(u32 index, u32 *value) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_READ_GGS, > - index, 0, value); > + return zynqmp_pm_ioctl(0, IOCTL_READ_GGS, index, 0, value); > } > EXPORT_SYMBOL_GPL(zynqmp_pm_read_ggs); > > @@ -661,8 +737,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_read_ggs); > */ > int zynqmp_pm_write_pggs(u32 index, u32 value) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_WRITE_PGGS, index, value, > - NULL); > + return zynqmp_pm_ioctl(0, IOCTL_WRITE_PGGS, index, value, NULL); > } > EXPORT_SYMBOL_GPL(zynqmp_pm_write_pggs); > > @@ -678,8 +753,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_write_pggs); > */ > int zynqmp_pm_read_pggs(u32 index, u32 *value) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_READ_PGGS, index, 0, > - value); > + return zynqmp_pm_ioctl(0, IOCTL_READ_PGGS, index, 0, value); > } > EXPORT_SYMBOL_GPL(zynqmp_pm_read_pggs); > > @@ -694,8 +768,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_read_pggs); > */ > int zynqmp_pm_set_boot_health_status(u32 value) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_SET_BOOT_HEALTH_STATUS, > - value, 0, NULL); > + return zynqmp_pm_ioctl(0, IOCTL_SET_BOOT_HEALTH_STATUS, value, 0, NULL); > } > > /** > -- > 2.7.4 > > This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately. Oops, this means I have to drop this, it's not compatible with kernel development, sorry. greg k-h _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH 1/3] firmware: xilinx: Add validation check for IOCTL 2020-09-16 11:33 ` Greg KH @ 2020-09-16 16:53 ` Amit Sunil Dhamne 0 siblings, 0 replies; 7+ messages in thread From: Amit Sunil Dhamne @ 2020-09-16 16:53 UTC (permalink / raw) To: Greg KH Cc: keescook, ard.biesheuvel, matt, dmitry.torokhov, linux-kernel, Jolly Shah, Rajan Vaja, Michal Simek, Tejas Patel, sudeep.holla, mingo, linux-arm-kernel, hkallweit1 Hi Greg, > -----Original Message----- > From: Greg KH <gregkh@linuxfoundation.org> > Sent: Wednesday, September 16, 2020 4:34 AM > To: Amit Sunil Dhamne <amitsuni@xilinx.com> > Cc: ard.biesheuvel@linaro.org; mingo@kernel.org; > matt@codeblueprint.co.uk; sudeep.holla@arm.com; hkallweit1@gmail.com; > keescook@chromium.org; dmitry.torokhov@gmail.com; Michal Simek > <michals@xilinx.com>; Rajan Vaja <RAJANV@xilinx.com>; Tejas Patel > <TEJASP@xilinx.com>; Jolly Shah <JOLLYS@xilinx.com>; linux-arm- > kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Rajan Vaja > <RAJANV@xilinx.com>; Jolly Shah <JOLLYS@xilinx.com> > Subject: Re: [PATCH 1/3] firmware: xilinx: Add validation check for IOCTL > > On Wed, Sep 09, 2020 at 05:20:02PM -0700, Amit Sunil Dhamne wrote: > > From: Tejas Patel <tejas.patel@xilinx.com> > > > > Validate IOCTL ID for ZynqMP and Versal before calling > > zynqmp_pm_invoke_fn(). > > > > Signed-off-by: Tejas Patel <tejas.patel@xilinx.com> > > Signed-off-by: Amit Sunil Dhamne <amit.sunil.dhamne@xilinx.com> > > --- > > drivers/firmware/xilinx/zynqmp.c | 117 > > +++++++++++++++++++++++++++++++-------- > > 1 file changed, 95 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/firmware/xilinx/zynqmp.c > > b/drivers/firmware/xilinx/zynqmp.c > > index 8d1ff24..8fe0912 100644 > > --- a/drivers/firmware/xilinx/zynqmp.c > > +++ b/drivers/firmware/xilinx/zynqmp.c > > @@ -514,6 +514,89 @@ int zynqmp_pm_clock_getparent(u32 clock_id, u32 > > *parent_id) EXPORT_SYMBOL_GPL(zynqmp_pm_clock_getparent); > > > > /** > > + * versal_is_valid_ioctl() - Check whether IOCTL ID is valid or not > > +for versal > > + * @ioctl_id: IOCTL ID > > + * > > + * Return: 1 if IOCTL is valid else 0 */ static inline int > > +versal_is_valid_ioctl(u32 ioctl_id) { > > + switch (ioctl_id) { > > + case IOCTL_SD_DLL_RESET: > > + case IOCTL_SET_SD_TAPDELAY: > > + case IOCTL_SET_PLL_FRAC_MODE: > > + case IOCTL_GET_PLL_FRAC_MODE: > > + case IOCTL_SET_PLL_FRAC_DATA: > > + case IOCTL_GET_PLL_FRAC_DATA: > > + case IOCTL_WRITE_GGS: > > + case IOCTL_READ_GGS: > > + case IOCTL_WRITE_PGGS: > > + case IOCTL_READ_PGGS: > > + case IOCTL_SET_BOOT_HEALTH_STATUS: > > + return 1; > > + default: > > + return 0; > > bool is nicer, right? > > > + } > > +} > > + > > +/** > > + * zynqmp_is_valid_ioctl() - Check whether IOCTL ID is valid or not > > + * @ioctl_id: IOCTL ID > > + * > > + * Return: 1 if IOCTL is valid else 0 */ static inline int > > +zynqmp_is_valid_ioctl(u32 ioctl_id) { > > + switch (ioctl_id) { > > + case IOCTL_SD_DLL_RESET: > > + case IOCTL_SET_SD_TAPDELAY: > > + case IOCTL_SET_PLL_FRAC_MODE: > > + case IOCTL_GET_PLL_FRAC_MODE: > > + case IOCTL_SET_PLL_FRAC_DATA: > > + case IOCTL_GET_PLL_FRAC_DATA: > > + case IOCTL_WRITE_GGS: > > + case IOCTL_READ_GGS: > > + case IOCTL_WRITE_PGGS: > > + case IOCTL_READ_PGGS: > > + case IOCTL_SET_BOOT_HEALTH_STATUS: > > + return 1; > > + default: > > + return 0; > > + } > > +} > > + > > +/** > > + * zynqmp_pm_ioctl() - PM IOCTL API for device control and configs > > + * @node_id: Node ID of the device > > + * @ioctl_id: ID of the requested IOCTL > > + * @arg1: Argument 1 to requested IOCTL call > > + * @arg2: Argument 2 to requested IOCTL call > > + * @out: Returned output value > > + * > > + * This function calls IOCTL to firmware for device control and > configuration. > > + * > > + * Return: Returns status, either success or error+reason */ static > > +int zynqmp_pm_ioctl(u32 node_id, u32 ioctl_id, u32 arg1, u32 arg2, > > + u32 *out) > > +{ > > + struct device_node *np; > > + > > + np = of_find_compatible_node(NULL, NULL, "xlnx,versal"); > > + if (np) { > > + if (!versal_is_valid_ioctl(ioctl_id)) > > + return -EINVAL; > > Wrong error value. > > > + } else { > > + if (!zynqmp_is_valid_ioctl(ioctl_id)) > > + return -EINVAL; > > Wrong error value. > > > + } > > + of_node_put(np); > > + > > + return zynqmp_pm_invoke_fn(PM_IOCTL, node_id, ioctl_id, arg1, > arg2, > > + out); > > No other checking of ioctl commands and arguments? Brave... > > > +} > > + > > +/** > > * zynqmp_pm_set_pll_frac_mode() - PM API for set PLL mode > > * > > * @clk_id: PLL clock ID > > @@ -525,8 +608,7 @@ > EXPORT_SYMBOL_GPL(zynqmp_pm_clock_getparent); > > */ > > int zynqmp_pm_set_pll_frac_mode(u32 clk_id, u32 mode) { > > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, > IOCTL_SET_PLL_FRAC_MODE, > > - clk_id, mode, NULL); > > + return zynqmp_pm_ioctl(0, IOCTL_SET_PLL_FRAC_MODE, clk_id, > > + mode, NULL); > > } > > EXPORT_SYMBOL_GPL(zynqmp_pm_set_pll_frac_mode); > > > > @@ -542,8 +624,7 @@ > EXPORT_SYMBOL_GPL(zynqmp_pm_set_pll_frac_mode); > > */ > > int zynqmp_pm_get_pll_frac_mode(u32 clk_id, u32 *mode) { > > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, > IOCTL_GET_PLL_FRAC_MODE, > > - clk_id, 0, mode); > > + return zynqmp_pm_ioctl(0, IOCTL_GET_PLL_FRAC_MODE, clk_id, 0, > > + mode); > > } > > EXPORT_SYMBOL_GPL(zynqmp_pm_get_pll_frac_mode); > > > > @@ -560,8 +641,7 @@ > EXPORT_SYMBOL_GPL(zynqmp_pm_get_pll_frac_mode); > > */ > > int zynqmp_pm_set_pll_frac_data(u32 clk_id, u32 data) { > > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, > IOCTL_SET_PLL_FRAC_DATA, > > - clk_id, data, NULL); > > + return zynqmp_pm_ioctl(0, IOCTL_SET_PLL_FRAC_DATA, clk_id, > > + data, NULL); > > } > > EXPORT_SYMBOL_GPL(zynqmp_pm_set_pll_frac_data); > > > > @@ -577,8 +657,7 @@ > EXPORT_SYMBOL_GPL(zynqmp_pm_set_pll_frac_data); > > */ > > int zynqmp_pm_get_pll_frac_data(u32 clk_id, u32 *data) { > > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, > IOCTL_GET_PLL_FRAC_DATA, > > - clk_id, 0, data); > > + return zynqmp_pm_ioctl(0, IOCTL_GET_PLL_FRAC_DATA, clk_id, 0, > > + data); > > } > > EXPORT_SYMBOL_GPL(zynqmp_pm_get_pll_frac_data); > > > > @@ -595,8 +674,8 @@ > EXPORT_SYMBOL_GPL(zynqmp_pm_get_pll_frac_data); > > */ > > int zynqmp_pm_set_sd_tapdelay(u32 node_id, u32 type, u32 value) { > > - return zynqmp_pm_invoke_fn(PM_IOCTL, node_id, > IOCTL_SET_SD_TAPDELAY, > > - type, value, NULL); > > + return zynqmp_pm_ioctl(node_id, IOCTL_SET_SD_TAPDELAY, type, > value, > > + NULL); > > } > > EXPORT_SYMBOL_GPL(zynqmp_pm_set_sd_tapdelay); > > > > @@ -612,8 +691,7 @@ > EXPORT_SYMBOL_GPL(zynqmp_pm_set_sd_tapdelay); > > */ > > int zynqmp_pm_sd_dll_reset(u32 node_id, u32 type) { > > - return zynqmp_pm_invoke_fn(PM_IOCTL, node_id, > IOCTL_SET_SD_TAPDELAY, > > - type, 0, NULL); > > + return zynqmp_pm_ioctl(node_id, IOCTL_SET_SD_TAPDELAY, type, > > + 0, NULL); > > } > > EXPORT_SYMBOL_GPL(zynqmp_pm_sd_dll_reset); > > > > @@ -628,8 +706,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_sd_dll_reset); > > */ > > int zynqmp_pm_write_ggs(u32 index, u32 value) { > > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_WRITE_GGS, > > - index, value, NULL); > > + return zynqmp_pm_ioctl(0, IOCTL_WRITE_GGS, index, value, > > + NULL); > > } > > EXPORT_SYMBOL_GPL(zynqmp_pm_write_ggs); > > > > @@ -644,8 +721,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_write_ggs); > > */ > > int zynqmp_pm_read_ggs(u32 index, u32 *value) { > > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_READ_GGS, > > - index, 0, value); > > + return zynqmp_pm_ioctl(0, IOCTL_READ_GGS, index, 0, value); > > } > > EXPORT_SYMBOL_GPL(zynqmp_pm_read_ggs); > > > > @@ -661,8 +737,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_read_ggs); > > */ > > int zynqmp_pm_write_pggs(u32 index, u32 value) { > > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_WRITE_PGGS, > index, value, > > - NULL); > > + return zynqmp_pm_ioctl(0, IOCTL_WRITE_PGGS, index, value, > > + NULL); > > } > > EXPORT_SYMBOL_GPL(zynqmp_pm_write_pggs); > > > > @@ -678,8 +753,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_write_pggs); > > */ > > int zynqmp_pm_read_pggs(u32 index, u32 *value) { > > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_READ_PGGS, > index, 0, > > - value); > > + return zynqmp_pm_ioctl(0, IOCTL_READ_PGGS, index, 0, value); > > } > > EXPORT_SYMBOL_GPL(zynqmp_pm_read_pggs); > > > > @@ -694,8 +768,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_read_pggs); > > */ > > int zynqmp_pm_set_boot_health_status(u32 value) { > > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, > IOCTL_SET_BOOT_HEALTH_STATUS, > > - value, 0, NULL); > > + return zynqmp_pm_ioctl(0, IOCTL_SET_BOOT_HEALTH_STATUS, > value, > > + 0, NULL); > > } > > > > /** > > -- > > 2.7.4 > > > > This email and any attachments are intended for the sole use of the named > recipient(s) and contain(s) confidential information that may be proprietary, > privileged or copyrighted under applicable law. If you are not the intended > recipient, do not read, copy, or forward this email message or any > attachments. Delete this email message and any attachments immediately. > > Oops, this means I have to drop this, it's not compatible with kernel > development, sorry. > > greg k-h I will incorporate your suggestions in the next version of patch-set. The footer message has unfortunately cropped up. Will also fix that in the next version. Thanks, Amit _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] firmware: xilinx: Add validation check for IOCTL 2020-09-10 0:20 ` [PATCH 1/3] firmware: xilinx: Add validation check for IOCTL Amit Sunil Dhamne 2020-09-16 11:33 ` Greg KH @ 2020-09-17 6:45 ` Punit Agrawal 1 sibling, 0 replies; 7+ messages in thread From: Punit Agrawal @ 2020-09-17 6:45 UTC (permalink / raw) To: Amit Sunil Dhamne Cc: tejas.patel, keescook, rajan.vaja, ard.biesheuvel, matt, gregkh, dmitry.torokhov, michal.simek, linux-kernel, jollys, rajanv, sudeep.holla, jolly.shah, mingo, linux-arm-kernel, hkallweit1 Hi Amit, Amit Sunil Dhamne <amit.sunil.dhamne@xilinx.com> writes: > From: Tejas Patel <tejas.patel@xilinx.com> > > Validate IOCTL ID for ZynqMP and Versal before calling > zynqmp_pm_invoke_fn(). > > Signed-off-by: Tejas Patel <tejas.patel@xilinx.com> > Signed-off-by: Amit Sunil Dhamne <amit.sunil.dhamne@xilinx.com> > --- > drivers/firmware/xilinx/zynqmp.c | 117 +++++++++++++++++++++++++++++++-------- > 1 file changed, 95 insertions(+), 22 deletions(-) > > diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c > index 8d1ff24..8fe0912 100644 > --- a/drivers/firmware/xilinx/zynqmp.c > +++ b/drivers/firmware/xilinx/zynqmp.c > @@ -514,6 +514,89 @@ int zynqmp_pm_clock_getparent(u32 clock_id, u32 *parent_id) > EXPORT_SYMBOL_GPL(zynqmp_pm_clock_getparent); > > /** > + * versal_is_valid_ioctl() - Check whether IOCTL ID is valid or not for versal > + * @ioctl_id: IOCTL ID > + * > + * Return: 1 if IOCTL is valid else 0 > + */ > +static inline int versal_is_valid_ioctl(u32 ioctl_id) > +{ > + switch (ioctl_id) { > + case IOCTL_SD_DLL_RESET: > + case IOCTL_SET_SD_TAPDELAY: > + case IOCTL_SET_PLL_FRAC_MODE: > + case IOCTL_GET_PLL_FRAC_MODE: > + case IOCTL_SET_PLL_FRAC_DATA: > + case IOCTL_GET_PLL_FRAC_DATA: > + case IOCTL_WRITE_GGS: > + case IOCTL_READ_GGS: > + case IOCTL_WRITE_PGGS: > + case IOCTL_READ_PGGS: > + case IOCTL_SET_BOOT_HEALTH_STATUS: > + return 1; > + default: > + return 0; > + } > +} > + > +/** > + * zynqmp_is_valid_ioctl() - Check whether IOCTL ID is valid or not > + * @ioctl_id: IOCTL ID > + * > + * Return: 1 if IOCTL is valid else 0 > + */ > +static inline int zynqmp_is_valid_ioctl(u32 ioctl_id) > +{ > + switch (ioctl_id) { > + case IOCTL_SD_DLL_RESET: > + case IOCTL_SET_SD_TAPDELAY: > + case IOCTL_SET_PLL_FRAC_MODE: > + case IOCTL_GET_PLL_FRAC_MODE: > + case IOCTL_SET_PLL_FRAC_DATA: > + case IOCTL_GET_PLL_FRAC_DATA: > + case IOCTL_WRITE_GGS: > + case IOCTL_READ_GGS: > + case IOCTL_WRITE_PGGS: > + case IOCTL_READ_PGGS: > + case IOCTL_SET_BOOT_HEALTH_STATUS: Other than the additional IOCTL being added in the next patch, this list is common between the two SoCs this driver runs on. To avoid duplication , it would be better to refactor into common and versal specific checks with the latter using the common function for shared IOCTLs. e.g., common_is_valid_ioctl() containing the common parts versal_is_valid_ioctl() using the above function One more comment below. > + return 1; > + default: > + return 0; > + } > +} > + > +/** > + * zynqmp_pm_ioctl() - PM IOCTL API for device control and configs > + * @node_id: Node ID of the device > + * @ioctl_id: ID of the requested IOCTL > + * @arg1: Argument 1 to requested IOCTL call > + * @arg2: Argument 2 to requested IOCTL call > + * @out: Returned output value > + * > + * This function calls IOCTL to firmware for device control and configuration. > + * > + * Return: Returns status, either success or error+reason > + */ > +static int zynqmp_pm_ioctl(u32 node_id, u32 ioctl_id, u32 arg1, u32 arg2, > + u32 *out) > +{ > + struct device_node *np; > + > + np = of_find_compatible_node(NULL, NULL, "xlnx,versal"); The driver is going to be probed either on a zyncmp or a versal based platform. You may want to consider stashing the result of the current platform at driver probe time and avoid searching through the device tree for every call. Thanks, Punit > + if (np) { > + if (!versal_is_valid_ioctl(ioctl_id)) > + return -EINVAL; > + } else { > + if (!zynqmp_is_valid_ioctl(ioctl_id)) > + return -EINVAL; > + } > + of_node_put(np); > + > + return zynqmp_pm_invoke_fn(PM_IOCTL, node_id, ioctl_id, arg1, arg2, > + out); > +} > + > +/** > * zynqmp_pm_set_pll_frac_mode() - PM API for set PLL mode > * > * @clk_id: PLL clock ID > @@ -525,8 +608,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_clock_getparent); > */ > int zynqmp_pm_set_pll_frac_mode(u32 clk_id, u32 mode) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_SET_PLL_FRAC_MODE, > - clk_id, mode, NULL); > + return zynqmp_pm_ioctl(0, IOCTL_SET_PLL_FRAC_MODE, clk_id, mode, NULL); > } > EXPORT_SYMBOL_GPL(zynqmp_pm_set_pll_frac_mode); > > @@ -542,8 +624,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_set_pll_frac_mode); > */ > int zynqmp_pm_get_pll_frac_mode(u32 clk_id, u32 *mode) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_GET_PLL_FRAC_MODE, > - clk_id, 0, mode); > + return zynqmp_pm_ioctl(0, IOCTL_GET_PLL_FRAC_MODE, clk_id, 0, mode); > } > EXPORT_SYMBOL_GPL(zynqmp_pm_get_pll_frac_mode); > > @@ -560,8 +641,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_get_pll_frac_mode); > */ > int zynqmp_pm_set_pll_frac_data(u32 clk_id, u32 data) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_SET_PLL_FRAC_DATA, > - clk_id, data, NULL); > + return zynqmp_pm_ioctl(0, IOCTL_SET_PLL_FRAC_DATA, clk_id, data, NULL); > } > EXPORT_SYMBOL_GPL(zynqmp_pm_set_pll_frac_data); > > @@ -577,8 +657,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_set_pll_frac_data); > */ > int zynqmp_pm_get_pll_frac_data(u32 clk_id, u32 *data) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_GET_PLL_FRAC_DATA, > - clk_id, 0, data); > + return zynqmp_pm_ioctl(0, IOCTL_GET_PLL_FRAC_DATA, clk_id, 0, data); > } > EXPORT_SYMBOL_GPL(zynqmp_pm_get_pll_frac_data); > > @@ -595,8 +674,8 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_get_pll_frac_data); > */ > int zynqmp_pm_set_sd_tapdelay(u32 node_id, u32 type, u32 value) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, node_id, IOCTL_SET_SD_TAPDELAY, > - type, value, NULL); > + return zynqmp_pm_ioctl(node_id, IOCTL_SET_SD_TAPDELAY, type, value, > + NULL); > } > EXPORT_SYMBOL_GPL(zynqmp_pm_set_sd_tapdelay); > > @@ -612,8 +691,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_set_sd_tapdelay); > */ > int zynqmp_pm_sd_dll_reset(u32 node_id, u32 type) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, node_id, IOCTL_SET_SD_TAPDELAY, > - type, 0, NULL); > + return zynqmp_pm_ioctl(node_id, IOCTL_SET_SD_TAPDELAY, type, 0, NULL); > } > EXPORT_SYMBOL_GPL(zynqmp_pm_sd_dll_reset); > > @@ -628,8 +706,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_sd_dll_reset); > */ > int zynqmp_pm_write_ggs(u32 index, u32 value) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_WRITE_GGS, > - index, value, NULL); > + return zynqmp_pm_ioctl(0, IOCTL_WRITE_GGS, index, value, NULL); > } > EXPORT_SYMBOL_GPL(zynqmp_pm_write_ggs); > > @@ -644,8 +721,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_write_ggs); > */ > int zynqmp_pm_read_ggs(u32 index, u32 *value) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_READ_GGS, > - index, 0, value); > + return zynqmp_pm_ioctl(0, IOCTL_READ_GGS, index, 0, value); > } > EXPORT_SYMBOL_GPL(zynqmp_pm_read_ggs); > > @@ -661,8 +737,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_read_ggs); > */ > int zynqmp_pm_write_pggs(u32 index, u32 value) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_WRITE_PGGS, index, value, > - NULL); > + return zynqmp_pm_ioctl(0, IOCTL_WRITE_PGGS, index, value, NULL); > } > EXPORT_SYMBOL_GPL(zynqmp_pm_write_pggs); > > @@ -678,8 +753,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_write_pggs); > */ > int zynqmp_pm_read_pggs(u32 index, u32 *value) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_READ_PGGS, index, 0, > - value); > + return zynqmp_pm_ioctl(0, IOCTL_READ_PGGS, index, 0, value); > } > EXPORT_SYMBOL_GPL(zynqmp_pm_read_pggs); > > @@ -694,8 +768,7 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_read_pggs); > */ > int zynqmp_pm_set_boot_health_status(u32 value) > { > - return zynqmp_pm_invoke_fn(PM_IOCTL, 0, IOCTL_SET_BOOT_HEALTH_STATUS, > - value, 0, NULL); > + return zynqmp_pm_ioctl(0, IOCTL_SET_BOOT_HEALTH_STATUS, value, 0, NULL); > } > > /** > -- > 2.7.4 > > This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately. > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/3] firmware: xilinx: Add support for GET_LAST_RESET_REASON IOCTL 2020-09-10 0:20 [PATCH 0/3] firmware: xilinx: Add support to get last reset reason Amit Sunil Dhamne 2020-09-10 0:20 ` [PATCH 1/3] firmware: xilinx: Add validation check for IOCTL Amit Sunil Dhamne @ 2020-09-10 0:20 ` Amit Sunil Dhamne 2020-09-10 0:20 ` [PATCH 3/3] firmware: xilinx: Add sysfs to get last reset reason Amit Sunil Dhamne 2 siblings, 0 replies; 7+ messages in thread From: Amit Sunil Dhamne @ 2020-09-10 0:20 UTC (permalink / raw) To: ard.biesheuvel, mingo, gregkh, matt, sudeep.holla, hkallweit1, keescook, dmitry.torokhov, michal.simek, rajanv, tejas.patel, jollys Cc: jolly.shah, rajan.vaja, Amit Sunil Dhamne, linux-kernel, linux-arm-kernel From: Tejas Patel <tejas.patel@xilinx.com> Add support to get last reason using IOCTL API. Also validate IOCTL ID for Versal or ZynqMP platforms before calling zynqmp_pm_invoke_fn(). Signed-off-by: Tejas Patel <tejas.patel@xilinx.com> Signed-off-by: Amit Sunil Dhamne <amit.sunil.dhamne@xilinx.com> --- drivers/firmware/xilinx/zynqmp.c | 20 ++++++++++++++++++++ include/linux/firmware/xlnx-zynqmp.h | 7 +++++++ 2 files changed, 27 insertions(+) diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c index 8fe0912..c5db34f 100644 --- a/drivers/firmware/xilinx/zynqmp.c +++ b/drivers/firmware/xilinx/zynqmp.c @@ -533,6 +533,7 @@ static inline int versal_is_valid_ioctl(u32 ioctl_id) case IOCTL_WRITE_PGGS: case IOCTL_READ_PGGS: case IOCTL_SET_BOOT_HEALTH_STATUS: + case IOCTL_GET_LAST_RESET_REASON: return 1; default: return 0; @@ -772,6 +773,25 @@ int zynqmp_pm_set_boot_health_status(u32 value) } /** + * zynqmp_pm_get_last_reset_reason() - PM API for getting last reset reason + * + * @reset_reason: last reset reason + * + * This function returns last reset reason + * + * Return: Returns status, either success or error+reason + */ +int zynqmp_pm_get_last_reset_reason(u32 *reset_reason) +{ + if (!reset_reason) + return -EINVAL; + + return zynqmp_pm_ioctl(0, IOCTL_GET_LAST_RESET_REASON, 0, 0, + reset_reason); +} +EXPORT_SYMBOL_GPL(zynqmp_pm_get_last_reset_reason); + +/** * zynqmp_pm_reset_assert - Request setting of reset (1 - assert, 0 - release) * @reset: Reset to be configured * @assert_flag: Flag stating should reset be asserted (1) or diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h index 5968df8..90c8664 100644 --- a/include/linux/firmware/xlnx-zynqmp.h +++ b/include/linux/firmware/xlnx-zynqmp.h @@ -116,6 +116,8 @@ enum pm_ioctl_id { IOCTL_READ_PGGS = 15, /* Set healthy bit value */ IOCTL_SET_BOOT_HEALTH_STATUS = 17, + /* IOCTL to get last reset reason */ + IOCTL_GET_LAST_RESET_REASON = 23, }; enum pm_query_id { @@ -357,6 +359,7 @@ int zynqmp_pm_write_pggs(u32 index, u32 value); int zynqmp_pm_read_pggs(u32 index, u32 *value); int zynqmp_pm_system_shutdown(const u32 type, const u32 subtype); int zynqmp_pm_set_boot_health_status(u32 value); +int zynqmp_pm_get_last_reset_reason(u32 *reset_reason); #else static inline struct zynqmp_eemi_ops *zynqmp_pm_get_eemi_ops(void) { @@ -507,6 +510,10 @@ static inline int zynqmp_pm_set_boot_health_status(u32 value) { return -ENODEV; } +static inline int zynqmp_pm_get_last_reset_reason(u32 *reset_reason) +{ + return -ENODEV; +} #endif #endif /* __FIRMWARE_ZYNQMP_H__ */ -- 2.7.4 This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] firmware: xilinx: Add sysfs to get last reset reason 2020-09-10 0:20 [PATCH 0/3] firmware: xilinx: Add support to get last reset reason Amit Sunil Dhamne 2020-09-10 0:20 ` [PATCH 1/3] firmware: xilinx: Add validation check for IOCTL Amit Sunil Dhamne 2020-09-10 0:20 ` [PATCH 2/3] firmware: xilinx: Add support for GET_LAST_RESET_REASON IOCTL Amit Sunil Dhamne @ 2020-09-10 0:20 ` Amit Sunil Dhamne 2 siblings, 0 replies; 7+ messages in thread From: Amit Sunil Dhamne @ 2020-09-10 0:20 UTC (permalink / raw) To: ard.biesheuvel, mingo, gregkh, matt, sudeep.holla, hkallweit1, keescook, dmitry.torokhov, michal.simek, rajanv, tejas.patel, jollys Cc: jolly.shah, rajan.vaja, Amit Sunil Dhamne, linux-kernel, linux-arm-kernel From: Tejas Patel <tejas.patel@xilinx.com> Add sysfs to get last reset reason. Signed-off-by: Tejas Patel <tejas.patel@xilinx.com> Signed-off-by: Amit Sunil Dhamne <amit.sunil.dhamne@xilinx.com> --- .../ABI/stable/sysfs-driver-firmware-zynqmp | 13 ++++++++ drivers/firmware/xilinx/zynqmp.c | 37 ++++++++++++++++++++++ include/linux/firmware/xlnx-zynqmp.h | 11 +++++++ 3 files changed, 61 insertions(+) diff --git a/Documentation/ABI/stable/sysfs-driver-firmware-zynqmp b/Documentation/ABI/stable/sysfs-driver-firmware-zynqmp index 00fa04c..96a5760 100644 --- a/Documentation/ABI/stable/sysfs-driver-firmware-zynqmp +++ b/Documentation/ABI/stable/sysfs-driver-firmware-zynqmp @@ -101,3 +101,16 @@ Description: # echo 0 > /sys/devices/platform/firmware\:zynqmp-firmware/health_status Users: Xilinx + +What: /sys/devices/platform/firmware\:zynqmp-firmware/last_reset_reason +Date: June 2020 +KernelVersion: 5.9.0 +Contact: "Tejas Patel" <tejasp@xilinx.com> +Description: + This sysfs interface allows to get last reset reason. + + Usage: + Get last reset reason + # cat /sys/devices/platform/firmware\:zynqmp-firmware/last_reset_reason + +Users: Xilinx diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c index c5db34f..f590856 100644 --- a/drivers/firmware/xilinx/zynqmp.c +++ b/drivers/firmware/xilinx/zynqmp.c @@ -1266,6 +1266,42 @@ static DEVICE_ATTR_RW(pggs1); static DEVICE_ATTR_RW(pggs2); static DEVICE_ATTR_RW(pggs3); +static ssize_t last_reset_reason_show(struct device *device, + struct device_attribute *attr, char *buf) +{ + int ret; + u32 ret_payload[PAYLOAD_ARG_CNT]; + + ret = zynqmp_pm_get_last_reset_reason(ret_payload); + if (-EINVAL == ret) + return sprintf(buf, "Feature not supported\n"); + else if (ret) + return ret; + + switch (ret_payload[1]) { + case PM_RESET_REASON_EXT_POR: + return sprintf(buf, "ext_por\n"); + case PM_RESET_REASON_SW_POR: + return sprintf(buf, "sw_por\n"); + case PM_RESET_REASON_SLR_POR: + return sprintf(buf, "sl_por\n"); + case PM_RESET_REASON_ERR_POR: + return sprintf(buf, "err_por\n"); + case PM_RESET_REASON_DAP_SRST: + return sprintf(buf, "dap_srst\n"); + case PM_RESET_REASON_ERR_SRST: + return sprintf(buf, "err_srst\n"); + case PM_RESET_REASON_SW_SRST: + return sprintf(buf, "sw_srst\n"); + case PM_RESET_REASON_SLR_SRST: + return sprintf(buf, "slr_srst\n"); + default: + return sprintf(buf, "unknown reset\n"); + } +} + +static DEVICE_ATTR_RO(last_reset_reason); + static struct attribute *zynqmp_firmware_attrs[] = { &dev_attr_ggs0.attr, &dev_attr_ggs1.attr, @@ -1277,6 +1313,7 @@ static struct attribute *zynqmp_firmware_attrs[] = { &dev_attr_pggs3.attr, &dev_attr_shutdown_scope.attr, &dev_attr_health_status.attr, + &dev_attr_last_reset_reason.attr, NULL, }; diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h index 90c8664..f7cb0a9 100644 --- a/include/linux/firmware/xlnx-zynqmp.h +++ b/include/linux/firmware/xlnx-zynqmp.h @@ -302,6 +302,17 @@ enum zynqmp_pm_shutdown_subtype { ZYNQMP_PM_SHUTDOWN_SUBTYPE_SYSTEM, }; +enum pm_reset_reason { + PM_RESET_REASON_EXT_POR = 0, + PM_RESET_REASON_SW_POR = 1, + PM_RESET_REASON_SLR_POR = 2, + PM_RESET_REASON_ERR_POR = 3, + PM_RESET_REASON_DAP_SRST = 7, + PM_RESET_REASON_ERR_SRST = 8, + PM_RESET_REASON_SW_SRST = 9, + PM_RESET_REASON_SLR_SRST = 10, +}; + /** * struct zynqmp_pm_query_data - PM query data * @qid: query ID -- 2.7.4 This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-09-17 6:47 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-10 0:20 [PATCH 0/3] firmware: xilinx: Add support to get last reset reason Amit Sunil Dhamne 2020-09-10 0:20 ` [PATCH 1/3] firmware: xilinx: Add validation check for IOCTL Amit Sunil Dhamne 2020-09-16 11:33 ` Greg KH 2020-09-16 16:53 ` Amit Sunil Dhamne 2020-09-17 6:45 ` Punit Agrawal 2020-09-10 0:20 ` [PATCH 2/3] firmware: xilinx: Add support for GET_LAST_RESET_REASON IOCTL Amit Sunil Dhamne 2020-09-10 0:20 ` [PATCH 3/3] firmware: xilinx: Add sysfs to get last reset reason Amit Sunil Dhamne
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).