On Tue 08 Mar 00:48 CST 2022, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@xxxxxxx>
>
> remote processor may support:
> - firmware recovery with help from main processor
> - self recovery without help from main processor
> - iommu
> - etc
>
> Introduce rproc features could simplify code to avoid adding more bool
> flags and let us optimize current code.
>
> Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
> ---
>
> V2:
> New
>
> include/linux/remoteproc.h | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
This is the API that other parts of the kernel use to interact with a
struct rproc, what feature flags do you have a need for other parts of
the kernel to be able to query?
> index 93a1d0050fbc..51edaf80692c 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -417,6 +417,7 @@ struct rproc_ops {
> * has attached to it
> * @RPROC_DETACHED: device has been booted by another entity and waiting
> * for the core to attach to it
> + * @RPROC_CRASHED_ATTACH_RECOVERY: device has crashed and self recovery
This seems to belong in the other patch...
> * @RPROC_LAST: just keep this one at the end
> *
> * Please note that the values of these states are used as indices
> @@ -489,6 +490,11 @@ struct rproc_dump_segment {
> loff_t offset;
> };
>
> +enum rproc_features {
> + RPROC_FEAT_ATTACH_RECOVERY = 0,
No need to specify that this is bit 0, and the enum will do that for you.
> + RPROC_MAX_FEATURES = 32,
You're using DECLARE_BITMAP() so why 32?
Regards,
Bjorn
> +};
> +
> /**
> * struct rproc - represents a physical remote processor device
> * @node: list node of this rproc object
> @@ -530,6 +536,7 @@ struct rproc_dump_segment {
> * @elf_machine: firmware ELF machine
> * @cdev: character device of the rproc
> * @cdev_put_on_release: flag to indicate if remoteproc should be shutdown on @char_dev release
> + * @features: indicate remoteproc features
> */
> struct rproc {
> struct list_head node;
> @@ -570,8 +577,19 @@ struct rproc {
> u16 elf_machine;
> struct cdev cdev;
> bool cdev_put_on_release;
> + DECLARE_BITMAP(features, RPROC_MAX_FEATURES);
> };
>
> +static inline bool rproc_has_feature(struct rproc *rproc, unsigned int feature)
> +{
> + return test_bit(feature, rproc->features);
> +}
> +
> +static inline void rproc_set_feature(struct rproc *rproc, unsigned int feature)
> +{
> + set_bit(feature, rproc->features);
> +}
> +
> /**
> * struct rproc_subdev - subdevice tied to a remoteproc
> * @node: list node related to the rproc subdevs list
> --
> 2.30.0
>
