| Message ID | 20240829012005.382715-3-bmasney@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | crypto: qcom-rng: fix support for ACPI-based systems | expand |
On Wed, Aug 28, 2024 at 09:20:05PM GMT, Brian Masney wrote: > The qcom-rng driver supports both ACPI and device tree based systems. > ACPI support was broken when the hw_random interface support was added. > Let's go ahead and fix this by checking has_acpi_companion(). > > This fix was boot tested on a Qualcomm Amberwing server. > > Fixes: f29cd5bb64c2 ("crypto: qcom-rng - Add hw_random interface support") > Signed-off-by: Brian Masney <bmasney@redhat.com> Please reorder the patches so that the Fix comes first (it can get backported to stable kernels). Renaming the field is a cleanup and as such it can come afterwards. > --- > drivers/crypto/qcom-rng.c | 36 ++++++++++++++++++++---------------- > 1 file changed, 20 insertions(+), 16 deletions(-) >
Hi, thanks for the patch. I have one doubt: On Wed, Aug 28, 2024 at 09:20:05PM -0400, Brian Masney wrote: > The qcom-rng driver supports both ACPI and device tree based systems. > ACPI support was broken when the hw_random interface support was added. > Let's go ahead and fix this by checking has_acpi_companion(). > > This fix was boot tested on a Qualcomm Amberwing server. > > Fixes: f29cd5bb64c2 ("crypto: qcom-rng - Add hw_random interface support") > Signed-off-by: Brian Masney <bmasney@redhat.com> > --- > drivers/crypto/qcom-rng.c | 36 ++++++++++++++++++++---------------- > 1 file changed, 20 insertions(+), 16 deletions(-) > > diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c > index 4ed545001b77..470062cb258c 100644 > --- a/drivers/crypto/qcom-rng.c > +++ b/drivers/crypto/qcom-rng.c > @@ -176,6 +176,21 @@ static struct rng_alg qcom_rng_alg = { > } > }; > > +static struct qcom_rng_match_data qcom_prng_match_data = { > + .skip_init = false, So with acpi, skip_init will be set to false now, right? But before f29cd5bb64c2 broke it, skip_init used to be set to true. Was that wrong before, or now? Ernesto > + .hwrng_support = false, > +}; > + > +static struct qcom_rng_match_data qcom_prng_ee_match_data = { > + .skip_init = true, > + .hwrng_support = false, > +}; > + > +static struct qcom_rng_match_data qcom_trng_match_data = { > + .skip_init = true, > + .hwrng_support = true, > +}; > + > static int qcom_rng_probe(struct platform_device *pdev) > { > struct qcom_rng *rng; > @@ -196,7 +211,11 @@ static int qcom_rng_probe(struct platform_device *pdev) > if (IS_ERR(rng->clk)) > return PTR_ERR(rng->clk); > > - rng->match_data = (struct qcom_rng_match_data *)of_device_get_match_data(&pdev->dev); > + if (has_acpi_companion(&pdev->dev)) > + rng->match_data = &qcom_prng_match_data; > + else > + rng->match_data = > + (struct qcom_rng_match_data *)of_device_get_match_data(&pdev->dev); > > qcom_rng_dev = rng; > ret = crypto_register_rng(&qcom_rng_alg); > @@ -231,21 +250,6 @@ static void qcom_rng_remove(struct platform_device *pdev) > qcom_rng_dev = NULL; > } > > -static struct qcom_rng_match_data qcom_prng_match_data = { > - .skip_init = false, > - .hwrng_support = false, > -}; > - > -static struct qcom_rng_match_data qcom_prng_ee_match_data = { > - .skip_init = true, > - .hwrng_support = false, > -}; > - > -static struct qcom_rng_match_data qcom_trng_match_data = { > - .skip_init = true, > - .hwrng_support = true, > -}; > - > static const struct acpi_device_id __maybe_unused qcom_rng_acpi_match[] = { > { .id = "QCOM8160", .driver_data = 1 }, > {} > -- > 2.46.0 >
On Thu, Aug 29, 2024 at 11:58 AM Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com> wrote: > thanks for the patch. I have one doubt: > > On Wed, Aug 28, 2024 at 09:20:05PM -0400, Brian Masney wrote: > > The qcom-rng driver supports both ACPI and device tree based systems. > > ACPI support was broken when the hw_random interface support was added. > > Let's go ahead and fix this by checking has_acpi_companion(). > > > > This fix was boot tested on a Qualcomm Amberwing server. > > > > Fixes: f29cd5bb64c2 ("crypto: qcom-rng - Add hw_random interface support") > > Signed-off-by: Brian Masney <bmasney@redhat.com> > > --- > > drivers/crypto/qcom-rng.c | 36 ++++++++++++++++++++---------------- > > 1 file changed, 20 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c > > index 4ed545001b77..470062cb258c 100644 > > --- a/drivers/crypto/qcom-rng.c > > +++ b/drivers/crypto/qcom-rng.c > > @@ -176,6 +176,21 @@ static struct rng_alg qcom_rng_alg = { > > } > > }; > > > > +static struct qcom_rng_match_data qcom_prng_match_data = { > > + .skip_init = false, > > So with acpi, skip_init will be set to false now, right? But before > f29cd5bb64c2 broke it, skip_init used to be set to true. Was that wrong > before, or now? Good catch! I didn't check the _DSD and assumed it was always false. I'll verify on the server we have in the lab and post a v2. Brian
diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c index 4ed545001b77..470062cb258c 100644 --- a/drivers/crypto/qcom-rng.c +++ b/drivers/crypto/qcom-rng.c @@ -176,6 +176,21 @@ static struct rng_alg qcom_rng_alg = { } }; +static struct qcom_rng_match_data qcom_prng_match_data = { + .skip_init = false, + .hwrng_support = false, +}; + +static struct qcom_rng_match_data qcom_prng_ee_match_data = { + .skip_init = true, + .hwrng_support = false, +}; + +static struct qcom_rng_match_data qcom_trng_match_data = { + .skip_init = true, + .hwrng_support = true, +}; + static int qcom_rng_probe(struct platform_device *pdev) { struct qcom_rng *rng; @@ -196,7 +211,11 @@ static int qcom_rng_probe(struct platform_device *pdev) if (IS_ERR(rng->clk)) return PTR_ERR(rng->clk); - rng->match_data = (struct qcom_rng_match_data *)of_device_get_match_data(&pdev->dev); + if (has_acpi_companion(&pdev->dev)) + rng->match_data = &qcom_prng_match_data; + else + rng->match_data = + (struct qcom_rng_match_data *)of_device_get_match_data(&pdev->dev); qcom_rng_dev = rng; ret = crypto_register_rng(&qcom_rng_alg); @@ -231,21 +250,6 @@ static void qcom_rng_remove(struct platform_device *pdev) qcom_rng_dev = NULL; } -static struct qcom_rng_match_data qcom_prng_match_data = { - .skip_init = false, - .hwrng_support = false, -}; - -static struct qcom_rng_match_data qcom_prng_ee_match_data = { - .skip_init = true, - .hwrng_support = false, -}; - -static struct qcom_rng_match_data qcom_trng_match_data = { - .skip_init = true, - .hwrng_support = true, -}; - static const struct acpi_device_id __maybe_unused qcom_rng_acpi_match[] = { { .id = "QCOM8160", .driver_data = 1 }, {}
The qcom-rng driver supports both ACPI and device tree based systems. ACPI support was broken when the hw_random interface support was added. Let's go ahead and fix this by checking has_acpi_companion(). This fix was boot tested on a Qualcomm Amberwing server. Fixes: f29cd5bb64c2 ("crypto: qcom-rng - Add hw_random interface support") Signed-off-by: Brian Masney <bmasney@redhat.com> --- drivers/crypto/qcom-rng.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-)