Hi Brad,
On Mo, 2022-06-13 at 12:56 -0700, Brad Larson wrote:
> From: Brad Larson <[email protected]>
>
> This patch adds the reset controller functionality for the
> AMD Pensando Elba System Resource Chip.
>
> Signed-off-by: Brad Larson <[email protected]>
[...]
> diff --git a/drivers/reset/reset-elbasr.c b/drivers/reset/reset-elbasr.c
> new file mode 100644
> index 000000000000..6e429cb11466
> --- /dev/null
> +++ b/drivers/reset/reset-elbasr.c
> @@ -0,0 +1,94 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2022 AMD Pensando
> + */
> +
> +#include <linux/mfd/pensando-elbasr.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/regmap.h>
> +#include <linux/err.h>
> +#include <linux/of.h>
> +
> +#include <dt-bindings/reset/amd,pensando-elba-reset.h>
> +
> +struct elbasr_reset {
> + struct reset_controller_dev rcdev;
> + struct regmap *regmap;
> +};
> +
> +static inline struct elbasr_reset *to_elbasr_rst(struct reset_controller_dev *rc)
> +{
> + return container_of(rc, struct elbasr_reset, rcdev);
> +}
> +
> +static inline int elbasr_reset_shift(unsigned long id)
> +{
> + switch (id) {
> + case EMMC_HW_RESET:
Are there more reset controls than EMMC_HW_RESET?
If so, please list them all.
If not, why is this a function with a switch statement for a single
reset bit?
> + return 6;
> + default:
> + return -EINVAL;
The error return value is never checked.
This can't be reached, since ELBASR_NR_RESETS == 1. So id will only
ever be 0.
> +static int elbasr_reset_probe(struct platform_device *pdev)
> +{
> + struct elbasr_data *elbasr = dev_get_drvdata(pdev->dev.parent);
Peeking into the MFD driver's private data structure seems unnecessary.
Consider using dev_get_regmap() instead.
regards
Philipp