On Wed, Apr 29, 2015 at 08:04:03PM +0200, Michal Suchanek wrote:
> The claim function assigns sram to cpu which when called from the emac
> driver causes exactly the issue the call is supposed to prevent: emac
> does not work because it does not have sram available.
> 
> Add additional parameter to the sunxi_sram_claim to determine if the
> sram is claimed for cpu or device.
> 
> Signed-off-by: Michal Suchanek <[email protected]>
> ---
>  drivers/net/ethernet/allwinner/sun4i-emac.c |  2 +-
>  drivers/soc/sunxi/sunxi_sram.c              | 11 ++++++++---
>  include/linux/soc/sunxi/sunxi_sram.h        |  2 +-
>  3 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/allwinner/sun4i-emac.c 
> b/drivers/net/ethernet/allwinner/sun4i-emac.c
> index 0d309a0..11e8dee 100644
> --- a/drivers/net/ethernet/allwinner/sun4i-emac.c
> +++ b/drivers/net/ethernet/allwinner/sun4i-emac.c
> @@ -859,7 +859,7 @@ static int emac_probe(struct platform_device *pdev)
>  
>       clk_prepare_enable(db->clk);
>  
> -     ret = sunxi_sram_claim(SUNXI_SRAM_EMAC, "emac");
> +     ret = sunxi_sram_claim(SUNXI_SRAM_EMAC, "emac", 1);

This extra argument is completely redundant with the second one.

>       if (ret)
>               dev_warn(&pdev->dev, "Couldn't map SRAM to device\n");
>  
> diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c
> index 3d03e89..6e73f38 100644
> --- a/drivers/soc/sunxi/sunxi_sram.c
> +++ b/drivers/soc/sunxi/sunxi_sram.c
> @@ -109,7 +109,7 @@ static const struct file_operations sunxi_sram_fops = {
>       .release = single_release,
>  };
>  
> -int sunxi_sram_claim(enum sunxi_sram_type type, const char *function)
> +int sunxi_sram_claim(enum sunxi_sram_type type, const char *function, int 
> device)
>  {
>       struct sunxi_sram_desc *sram;
>       struct sunxi_sram_func *func;
> @@ -135,14 +135,19 @@ int sunxi_sram_claim(enum sunxi_sram_type type, const 
> char *function)
>  
>               sram->claimed = true;
>               spin_unlock(&sram_lock);
> -             dev_info(&pdev->dev, "Claiming sram %s.\n", sram->name );
> +             dev_info(&pdev->dev, "Claiming sram %s for %s.\n", sram->name,
> +                    device ? "device" : "cpu");
>  
>               for (func = sram->func; func->func; func++) {
>                       if (strcmp(function, func->func))
>                               continue;
>  

Ok, so, let's take and example with the EMAC SRAM.

sram->offset = 4
sram->width = 1

func->val = 1

>                       val = readl(base + sram->reg);

Let's say that val = 0xffffffff.

> -                     val &= ~GENMASK(sram->offset + sram->width,
> +                     if (device)
> +                             val |= GENMASK(sram->offset + sram->width,
> +                                     sram->offset);

val |= 0x100

    => 0xffffffff

> +                     else
> +                             val &= ~GENMASK(sram->offset + sram->width,
>                                       sram->offset);

val &= 0xfffff0ff

    => 0xfffffeff

Obviously, only the second case if you want to clear up that function,
which is what is done here.

>                       writel(val | func->val, base + sram->reg);

And here you actually write the bit.

Which will be val | func->val, 0 or 1 in our case. There's indeed a
bug, but the only thing that needs some fixing is that func->val needs
to be shifted by sram->offset.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Attachment: signature.asc
Description: Digital signature

Reply via email to