diff mbox

hwrng: pasemi_rng.c: Migrate to managed API

Message ID 1472124856-12767-1-git-send-email-prasannatsmkumar@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Herbert Xu
Headers show

Commit Message

PrasannaKumar Muralidharan Aug. 25, 2016, 11:34 a.m. UTC
Use devm_ioremap and devm_hwrng_register instead of ioremap and
hwrng_register. This removes unregistering and error handling code.

This patch is not tested with hardware as I don't have access to it.

Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
---
 drivers/char/hw_random/pasemi-rng.c | 26 +++-----------------------
 1 file changed, 3 insertions(+), 23 deletions(-)

Comments

Corentin Labbe Aug. 25, 2016, 12:15 p.m. UTC | #1
On Thu, Aug 25, 2016 at 05:04:16PM +0530, PrasannaKumar Muralidharan wrote:
> Use devm_ioremap and devm_hwrng_register instead of ioremap and
> hwrng_register. This removes unregistering and error handling code.
> 
> This patch is not tested with hardware as I don't have access to it.
> 
> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> ---
>  drivers/char/hw_random/pasemi-rng.c | 26 +++-----------------------
>  1 file changed, 3 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/char/hw_random/pasemi-rng.c b/drivers/char/hw_random/pasemi-rng.c
> index 699b725..0f03397 100644
> --- a/drivers/char/hw_random/pasemi-rng.c
> +++ b/drivers/char/hw_random/pasemi-rng.c
> @@ -100,37 +100,18 @@ static int rng_probe(struct platform_device *ofdev)
>  	void __iomem *rng_regs;
>  	struct device_node *rng_np = ofdev->dev.of_node;
>  	struct resource res;
> -	int err = 0;
>  
> -	err = of_address_to_resource(rng_np, 0, &res);
> -	if (err)
> +	if (of_address_to_resource(rng_np, 0, &res))
>  		return -ENODEV;
>  
> -	rng_regs = ioremap(res.start, 0x100);
> -
> +	rng_regs = devm_ioremap(&ofdev->dev, res.start, 0x100);
>  	if (!rng_regs)
>  		return -ENOMEM;
>  

I will propose to use devm_ioremap_resource() instead for removing this hardcoded 0x100, but i cannot find any user of this driver in any dts. (And so cannot check that this 0x100 is given in any DT resource node)

Is this normal ?

Regard

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
PrasannaKumar Muralidharan Aug. 25, 2016, 1:25 p.m. UTC | #2
> I will propose to use devm_ioremap_resource() instead for removing this hardcoded 0x100, but i cannot find any user of this driver in any dts. (And so cannot check that this 0x100 is given in any DT resource node)
>
> Is this normal ?

I wanted to use devm_ioremap_resource but could not find DT entry
required for this driver in any of the .dts files. So did not change
that. I could not find any dts/dtsi for this platform. So I assume
that the dtb is not present in the kernel, dtb is supplied by the
bootloader. I may be wrong in this. Can anyone confirm this?

Regards,
PrasannaKumar
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Darren Stevens Aug. 29, 2016, 5:52 p.m. UTC | #3
Hello PrasannaKumar

On 25/08/2016, PrasannaKumar Muralidharan wrote:
>> I will propose to use devm_ioremap_resource() instead for removing this
>> hardcoded 0x100, but i cannot find any user of this driver in any dts.
>> (And so cannot check that this 0x100 is given in any DT resource node)
>
>> Is this normal ?
>
> I wanted to use devm_ioremap_resource but could not find DT entry
> required for this driver in any of the .dts files. So did not change
> that. I could not find any dts/dtsi for this platform. So I assume
> that the dtb is not present in the kernel, dtb is supplied by the
> bootloader. I may be wrong in this. Can anyone confirm this?

On mine (Amigaone X1000) that is correct, we boot linux with a vmlinux file,
and the bootloader (CFE) passes a fixed dtb. I think it is possible to dump
the tree from inside CFE, if it would help I can invetigate?

Regards
Darren

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
PrasannaKumar Muralidharan Aug. 30, 2016, 7:16 a.m. UTC | #4
Hi Darren,

>> I wanted to use devm_ioremap_resource but could not find DT entry
>> required for this driver in any of the .dts files. So did not change
>> that. I could not find any dts/dtsi for this platform. So I assume
>> that the dtb is not present in the kernel, dtb is supplied by the
>> bootloader. I may be wrong in this. Can anyone confirm this?
>
> On mine (Amigaone X1000) that is correct, we boot linux with a vmlinux file,
> and the bootloader (CFE) passes a fixed dtb. I think it is possible to dump
> the tree from inside CFE, if it would help I can invetigate?

I don't know if it is possible to get dts from dtb even if you manage
to extract devicetree blob from your system.

Labbe, Do you know anyway to get dts from dtb? Is this step really
required to remove 0x100 value for this patch given that the value was
present here for years? If extracting dtb and converting dtb to dts is
easy and not time consuming, I am in favour of finding a way to remove
hard coded value.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Darren Stevens Aug. 31, 2016, 2:02 p.m. UTC | #5
Hello PrasannaKumar

On 30/08/2016, PrasannaKumar Muralidharan wrote:
> Hi Darren,
>> On mine (Amigaone X1000) that is correct, we boot linux with a vmlinux
>> file, and the bootloader (CFE) passes a fixed dtb. I think it is
>> possible to dump the tree from inside CFE, if it would help I can
>> invetigate?
>
> I don't know if it is possible to get dts from dtb even if you manage
> to extract devicetree blob from your system.

I didn't explain well, There is a CFE command 'show devtree' here's the
relevant bits (I Hope)

[CFE ]CFE> show devtree
[/]
 | #interrupt-cells                 val   0x00000002
 | #address-cells                   val   0x00000002
 | #size-cells                      val   0x00000002

...[snip]...

   [sdc@fc000000]
    | name                          str   'sdc' 
    | device_type                   str   'sdc' 
    | #address-cells                val   0x00000001
    | #size-cells                   val   0x00000001
    | compatible                    str   '1682m-sdc' 'pasemi,pwrficient-sdc'
'pasemi,sdc' 
    | reg                           cell  00000000 FC000000 00000000 00800000 

...[snip]...

      [rng@fc105000]
       | name                       str   'rng' 
       | device_type                str   'rng' 
       | compatible                 str   '1682m-rng' 'pasemi,pwrficient-rng'
'pasemi,rng' 
       | reg                        cell  FC105000 00001000 


Regards

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
PrasannaKumar Muralidharan Sept. 1, 2016, 6:59 a.m. UTC | #6
> I didn't explain well, There is a CFE command 'show devtree' here's the
> relevant bits (I Hope)

This is much simple than I expected.

> [CFE ]CFE> show devtree
> [/]
>  | #interrupt-cells                 val   0x00000002
>  | #address-cells                   val   0x00000002
>  | #size-cells                      val   0x00000002
>
> ...[snip]...
>
>    [sdc@fc000000]
>     | name                          str   'sdc'
>     | device_type                   str   'sdc'
>     | #address-cells                val   0x00000001
>     | #size-cells                   val   0x00000001
>     | compatible                    str   '1682m-sdc' 'pasemi,pwrficient-sdc'
> 'pasemi,sdc'
>     | reg                           cell  00000000 FC000000 00000000 00800000
>
> ...[snip]...
>
>       [rng@fc105000]
>        | name                       str   'rng'
>        | device_type                str   'rng'
>        | compatible                 str   '1682m-rng' 'pasemi,pwrficient-rng'
> 'pasemi,rng'
>        | reg                        cell  FC105000 00001000
>

Device tree provided by CFE has the size. I will send another patch
which uses devm_ioremap_resource().

Appreciate your help. Thank you :).
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/char/hw_random/pasemi-rng.c b/drivers/char/hw_random/pasemi-rng.c
index 699b725..0f03397 100644
--- a/drivers/char/hw_random/pasemi-rng.c
+++ b/drivers/char/hw_random/pasemi-rng.c
@@ -100,37 +100,18 @@  static int rng_probe(struct platform_device *ofdev)
 	void __iomem *rng_regs;
 	struct device_node *rng_np = ofdev->dev.of_node;
 	struct resource res;
-	int err = 0;
 
-	err = of_address_to_resource(rng_np, 0, &res);
-	if (err)
+	if (of_address_to_resource(rng_np, 0, &res))
 		return -ENODEV;
 
-	rng_regs = ioremap(res.start, 0x100);
-
+	rng_regs = devm_ioremap(&ofdev->dev, res.start, 0x100);
 	if (!rng_regs)
 		return -ENOMEM;
 
 	pasemi_rng.priv = (unsigned long)rng_regs;
 
 	pr_info("Registering PA Semi RNG\n");
-
-	err = hwrng_register(&pasemi_rng);
-
-	if (err)
-		iounmap(rng_regs);
-
-	return err;
-}
-
-static int rng_remove(struct platform_device *dev)
-{
-	void __iomem *rng_regs = (void __iomem *)pasemi_rng.priv;
-
-	hwrng_unregister(&pasemi_rng);
-	iounmap(rng_regs);
-
-	return 0;
+	return devm_hwrng_register(&ofdev->dev, &pasemi_rng);
 }
 
 static const struct of_device_id rng_match[] = {
@@ -146,7 +127,6 @@  static struct platform_driver rng_driver = {
 		.of_match_table = rng_match,
 	},
 	.probe		= rng_probe,
-	.remove		= rng_remove,
 };
 
 module_platform_driver(rng_driver);