Re: [PATCH] mmc: mxs-mmc: Add support for non-removable cards

2013-04-04 Thread Marek Vasut
Dear Alexandre Pereira da Silva,

> Some boards and card slots doesn't have card detect feature available.
> In that case allow to mark the cards as non-removable, via devicetree.
> 
> Signed-off-by: Alexandre Pereira da Silva 

Looks good,

Reviewed-by: Marek Vasut 

btw. did you know you can let git send-email automatically handle CC for you so 
you don't have to type it into the command line by simply sticking

Cc: Us Er 

in the commit message? Preferably under the SoB line.

> ---
>  drivers/mmc/host/mxs-mmc.c |6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
> index 4efe302..7d2cd74 100644
> --- a/drivers/mmc/host/mxs-mmc.c
> +++ b/drivers/mmc/host/mxs-mmc.c
> @@ -95,6 +95,9 @@ static int mxs_mmc_get_cd(struct mmc_host *mmc)
>   struct mxs_mmc_host *host = mmc_priv(mmc);
>   struct mxs_ssp *ssp = &host->ssp;
> 
> + if (mmc->caps & MMC_CAP_NONREMOVABLE)
> + return 1;
> +
>   return !(readl(ssp->base + HW_SSP_STATUS(ssp)) &
>BM_SSP_STATUS_CARD_DETECT);
>  }
> @@ -691,6 +694,9 @@ static int mxs_mmc_probe(struct platform_device *pdev)
>   if (flags & OF_GPIO_ACTIVE_LOW)
>   host->wp_inverted = 1;
> 
> + if (of_find_property(np, "non-removable", NULL))
> + mmc->caps |= MMC_CAP_NONREMOVABLE;
> +
>   mmc->f_min = 40;
>   mmc->f_max = 28800;
>   mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM: dts: imx23-olinuxino: Set the sdcard as non-removable

2013-04-04 Thread Marek Vasut
Dear Alexandre Pereira da Silva,

> The imx23-olinuxino sdcard doesn't support card detect.
> 
> Signed-off-by: Alexandre Pereira da Silva 
> ---
>  arch/arm/boot/dts/imx23-olinuxino.dts |1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/imx23-olinuxino.dts
> b/arch/arm/boot/dts/imx23-olinuxino.dts index e7484e4..03f55c2 100644
> --- a/arch/arm/boot/dts/imx23-olinuxino.dts
> +++ b/arch/arm/boot/dts/imx23-olinuxino.dts
> @@ -29,6 +29,7 @@
>   pinctrl-names = "default";
>   pinctrl-0 = <&mmc0_4bit_pins_a 
&mmc0_pins_fixup>;
>   bus-width = <4>;
> + non-removable;
>   status = "okay";
>   };

The card on mx23 olinuxino seems perfectly removable to me ... on all possible 
models and even prototypes and hybrids too ;-)

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM: dts: imx23-olinuxino: Set the sdcard as non-removable

2013-04-05 Thread Marek Vasut
Dear Shawn Guo,

> On Fri, Apr 05, 2013 at 01:16:53AM -0300, Alexandre Pereira da Silva wrote:
> > The imx23-olinuxino sdcard doesn't support card detect.
> 
> So it sounds more like a "broken-cd" rather than "non-removable"?

The CD line is working perfectly well on this board, so I don't get it.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] mmc: mxs-mmc: Add support for non-removable cards

2013-04-05 Thread Marek Vasut
Dear Mark Brown,

> On Fri, Apr 05, 2013 at 08:37:39AM +0200, Marek Vasut wrote:
> > btw. did you know you can let git send-email automatically handle CC for
> > you so you don't have to type it into the command line by simply
> > sticking
> > 
> > Cc: Us Er 
> > 
> > in the commit message? Preferably under the SoB line.
> 
> Though not everyone likes seeing things like that end up in the commit
> log (or having to edit them out of commit logs).

Ah, all right.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] ARM: dts: imx23-olinuxino: Set the sdcard as non-removable

2013-04-05 Thread Marek Vasut
Dear Alexandre Pereira da Silva,

> On Fri, Apr 5, 2013 at 11:11 AM, Marek Vasut  wrote:
> > The CD line is working perfectly well on this board, so I don't get it.
> 
> In the schematics of this board, the SSP1_DETECT pin is connected to
> the green led. As I see, there is no way, the hardware can detect card
> removal. Am I missing something here?

You're right. Then the design is indeed defective. Sorry.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: linux-next: manual merge of the spi-mb tree with the mmc tree

2012-09-04 Thread Marek Vasut
Dear Stephen Rothwell,

> Hi Mark,
> 
> Today's linux-next merge of the spi-mb tree got a conflict in
> drivers/mmc/host/mxs-mmc.c between commit fc108d24d3a6 ("mmc: mxs-mmc:
> fix deadlock caused by recursion loop") from the mmc tree and commit
> 829c1bf40b92 ("mmc: spi: Pull out parts shared between MMC and SPI") from
> the spi-mb tree.
> 
> I fixed it up (see below) and can carry the fix as necessary (no action
> is required).

On a quick glance, it seems correct. Thank you!

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 04/17] ARM: pxa: define palmte2_pxa_keys conditionally

2012-10-03 Thread Marek Vasut
Dear Arnd Bergmann,

> Gcc prints a harmless warning about palmte2_pxa_keys not being used
> when the gpio keyboard driver is disabled. The solution is to use
> the same #ifdef that is already present in the place where the
> symbol is used.
> 
> Without this patch, building palmz72_defconfig results in:
> 
> /home/arnd/linux-arm/arch/arm/mach-pxa/palmte2.c:128:31: warning:
> 'palmte2_pxa_keys' defined but not used [-Wunused-variable]
> 
> Signed-off-by: Arnd Bergmann 
> Cc: Marek Vasut 
> Cc: Carlos Eduardo Medaglia Dyonisio 
> Cc: Haojian Zhuang 
> Cc: Eric Miao 

Acked-by: Marek Vasut 

Thanks Arnd

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/4] backlight: lms283gf05: add missing const

2012-10-23 Thread Marek Vasut
Dear Jingoo Han,

> Add 'const' to static array that was missing it in its
> definition.

Did you get compiler warning? Still, this is a good pick

Acked-by: Marek Vasut 

> Signed-off-by: Jingoo Han 
> Cc: Marek Vasut 
> Cc: Richard Purdie 
> ---
[...]

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH for 3.7] mtd: nand: fix Samsung SLC NAND identification regression

2012-10-31 Thread Marek Vasut
Dear Brian Norris,

> Hi David,
> 
> On Wed, Oct 10, 2012 at 12:48 AM, David Woodhouse  wrote:
> > On Tue, 2012-10-09 at 23:39 -0700, Brian Norris wrote:
> >> I can see if that's possible, but I think it's unlikely. They don't
> >> even bother following standards (ONFI). Is this an obstacle to
> >> merging?
> > 
> > No. I already pushed it.
> 
> This is a "bump" for a 3.7-rc pull request. This regression has been
> noticed by others.

[whine] my nand doesn't work ;-)

Thanks for keeping eye on this, Brian.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 04/12] drivers/mmc/host: don't use devm_pinctrl_get_select_default() in probe

2013-07-10 Thread Marek Vasut
Dear Wolfram Sang,

> Since commit ab78029 (drivers/pinctrl: grab default handles from device
> core), we can rely on device core for setting the default pins. Compile
> tested only.
> 
> Acked-by: Linus Walleij  (personally at LCE13)
> Signed-off-by: Wolfram Sang 

Something like that is already in -next:

mmc: mxs-mmc: Let device core handle pinctrl

Otherwise

Tested-by: Marek Vasut 

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: KSZ8051 breakage

2013-09-18 Thread Marek Vasut
Dear Alexandre Pereira da Silva,

> Hi,
> 
> The commit
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/driv
> ers/net/phy/micrel.c?id=b818d1a7f72575eef17e00dc4085512c9cc8897dbreaks
> KSZ8051MNL phy
> 
> Micrel gone wrong here. The PHY ID of KSZ8051 is the same as KSZ8031, even
> the revision code matches.
> 
> In my custom board, I cannot run these instructions or else my cpu will
> lock up:
> const u16 val = KSZPHY_OMSO_B_CAST_OFF | KSZPHY_OMSO_RMII_OVERRIDE;
> phy_write(phydev, MII_KSZPHY_OMSO, val);
> 
> This is a board property and not a phy one. Ideally, the phy should be
> correctly strapped at the hardware level for the board in question, so no
> changes in this registers should be needed.
> 
> I think we could add a OF property to set these options, what do you think?
> If that, I would also propose to convert the 50MHZ flag to OF as well.
> 
> Other option is to handle this the same way as the 50MHZ flag, that is
> being set by board code.

I wouldn't mind, but we should use something generic the other PHYs can use as 
well.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 5/7] regulator: palmas: Use devm_regulator_register

2013-09-04 Thread Marek Vasut
Dear Sachin Kamat,

> devm_* simplifies the code.
> 
> Signed-off-by: Sachin Kamat 
> ---
>  drivers/regulator/palmas-regulator.c |   38
> ++ 1 file changed, 11 insertions(+), 27
> deletions(-)
> 
> diff --git a/drivers/regulator/palmas-regulator.c
> b/drivers/regulator/palmas-regulator.c index d0c8785..37b1068 100644
> --- a/drivers/regulator/palmas-regulator.c
> +++ b/drivers/regulator/palmas-regulator.c
> @@ -852,7 +852,7 @@ static int palmas_regulators_probe(struct
> platform_device *pdev) if (ret < 0) {
>   dev_err(&pdev->dev,
>   "reading TSTEP reg failed: %d\n", ret);
> - goto err_unregister_regulator;
> + return ret;
>   }
>   pmic->desc[id].ramp_delay =
>   palmas_smps_ramp_delay[reg & 0x3];
> @@ -864,7 +864,7 @@ static int palmas_regulators_probe(struct
> platform_device *pdev) reg_init = pdata->reg_init[id];
>   ret = palmas_smps_init(palmas, id, reg_init);
>   if (ret)
> - goto err_unregister_regulator;
> + return ret;
>   }
> 
>   /* Register the regulators */
> @@ -897,7 +897,7 @@ static int palmas_regulators_probe(struct
> platform_device *pdev)
> 
>   ret = palmas_smps_read(pmic->palmas, addr, ®);
>   if (ret)
> - goto err_unregister_regulator;
> + return ret;
>   if (reg & PALMAS_SMPS12_VOLTAGE_RANGE)
>   pmic->range[id] = 1;
> 
> @@ -913,7 +913,7 @@ static int palmas_regulators_probe(struct
> platform_device *pdev) addr = palmas_regs_info[id].ctrl_addr;
>   ret = palmas_smps_read(pmic->palmas, addr, ®);
>   if (ret)
> - goto err_unregister_regulator;
> + return ret;
>   pmic->current_reg_mode[id] = reg &
>   PALMAS_SMPS12_CTRL_MODE_ACTIVE_MASK;
>   }
> @@ -929,13 +929,13 @@ static int palmas_regulators_probe(struct
> platform_device *pdev) pmic->desc[id].supply_name =
> palmas_regs_info[id].sname;
>   config.of_node = palmas_matches[id].of_node;
> 
> - rdev = regulator_register(&pmic->desc[id], &config);
> + rdev = devm_regulator_register(&pdev->dev, &pmic->desc[id],
> +&config);
>   if (IS_ERR(rdev)) {
>   dev_err(&pdev->dev,
>   "failed to register %s regulator\n",
>   pdev->name);
> - ret = PTR_ERR(rdev);
> - goto err_unregister_regulator;
> + return PTR_ERR(rdev);
>   }
> 
>   /* Save regulator for cleanup */
> @@ -997,13 +997,13 @@ static int palmas_regulators_probe(struct
> platform_device *pdev) pmic->desc[id].supply_name =
> palmas_regs_info[id].sname;
>   config.of_node = palmas_matches[id].of_node;
> 
> - rdev = regulator_register(&pmic->desc[id], &config);
> + rdev = devm_regulator_register(&pdev->dev, &pmic->desc[id],
> +&config);
>   if (IS_ERR(rdev)) {
>   dev_err(&pdev->dev,
>   "failed to register %s regulator\n",
>   pdev->name);
> - ret = PTR_ERR(rdev);
> - goto err_unregister_regulator;
> + return PTR_ERR(rdev);
>   }
> 
>   /* Save regulator for cleanup */
> @@ -1021,7 +1021,7 @@ static int palmas_regulators_probe(struct
> platform_device *pdev) id, reg_init);
>   if (ret) {
>   regulator_unregister(pmic->rdev[id]);
> - goto err_unregister_regulator;
> + return ret;
>   }

what about this regulator_unregister() above here?

[...]
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 6/7] regulator: rc5t583: Use devm_regulator_register

2013-09-04 Thread Marek Vasut
Dear Sachin Kamat,

> devm_* simplifies the code.
> 
> Signed-off-by: Sachin Kamat 
> ---
>  drivers/regulator/rc5t583-regulator.c |   22 ++
>  1 file changed, 2 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/regulator/rc5t583-regulator.c
> b/drivers/regulator/rc5t583-regulator.c index 5885b45..b58affb 100644
> --- a/drivers/regulator/rc5t583-regulator.c
> +++ b/drivers/regulator/rc5t583-regulator.c
> @@ -173,33 +173,16 @@ skip_ext_pwr_config:
>   config.driver_data = reg;
>   config.regmap = rc5t583->regmap;
> 
> - rdev = regulator_register(&ri->desc, &config);
> + rdev = devm_regulator_register(&pdev->dev, &ri->desc, &config);
>   if (IS_ERR(rdev)) {
>   dev_err(&pdev->dev, "Failed to register regulator %s\n",
>   ri->desc.name);
> - ret = PTR_ERR(rdev);
> - goto clean_exit;
> + return PTR_ERR(rdev);
>   }
>   reg->rdev = rdev;
>   }
>   platform_set_drvdata(pdev, regs);
>   return 0;
> -
> -clean_exit:
> - while (--id >= 0)
> - regulator_unregister(regs[id].rdev);
> -
> - return ret;
> -}
> -
> -static int rc5t583_regulator_remove(struct platform_device *pdev)
> -{
> - struct rc5t583_regulator *regs = platform_get_drvdata(pdev);
> - int id;
> -
> - for (id = 0; id < RC5T583_REGULATOR_MAX; ++id)
> - regulator_unregister(regs[id].rdev);
> - return 0;
>  }
> 
>  static struct platform_driver rc5t583_regulator_driver = {
> @@ -208,7 +191,6 @@ static struct platform_driver rc5t583_regulator_driver
> = { .owner= THIS_MODULE,
>   },
>   .probe  = rc5t583_regulator_probe,
> -     .remove = rc5t583_regulator_remove,
>  };
> 
>  static int __init rc5t583_regulator_init(void)

Next up, you might want to start flipping the regulator drivers to 
module_platform_driver ;-)

Acked-by: Marek Vasut 

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/7] regulator: anatop-regulator: Use devm_regulator_register

2013-09-04 Thread Marek Vasut
Dear Sachin Kamat,

> devm_* simplifies the code.
> 
> Signed-off-by: Sachin Kamat 

Makes sense

Acked-by: Marek Vasut 

> ---
> This series is compile tested.
> ---
>  drivers/regulator/anatop-regulator.c |3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/regulator/anatop-regulator.c
> b/drivers/regulator/anatop-regulator.c index 0d4a8cc..e42bfd1 100644
> --- a/drivers/regulator/anatop-regulator.c
> +++ b/drivers/regulator/anatop-regulator.c
> @@ -200,7 +200,7 @@ static int anatop_regulator_probe(struct
> platform_device *pdev) config.regmap = sreg->anatop;
> 
>   /* register regulator */
> - rdev = regulator_register(rdesc, &config);
> + rdev = devm_regulator_register(dev, rdesc, &config);
>   if (IS_ERR(rdev)) {
>   dev_err(dev, "failed to register %s\n",
>   rdesc->name);
> @@ -223,7 +223,6 @@ static int anatop_regulator_remove(struct
> platform_device *pdev) struct anatop_regulator *sreg =
> rdev_get_drvdata(rdev);
>   const char *name = sreg->name;
> 
> - regulator_unregister(rdev);
>   kfree(name);
> 
>   return 0;

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/7] regulator: mc13783: Use devm_regulator_register

2013-09-04 Thread Marek Vasut
Dear Sachin Kamat,

> devm_* simplifies the code.
> 
> Signed-off-by: Sachin Kamat 

Acked-by: Marek Vasut 

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 4/7] regulator: mc13892: Use devm_regulator_register

2013-09-04 Thread Marek Vasut
Dear Sachin Kamat,

> devm_* simplifies the code.
> 
> Signed-off-by: Sachin Kamat 

Acked-by: Marek Vasut 

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/7] regulator: isl6271a-regulator: Use devm_regulator_register

2013-09-04 Thread Marek Vasut
Dear Sachin Kamat,

> devm_* simplifies the code.
> 
> Signed-off-by: Sachin Kamat 

Acked-by: Marek Vasut 

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/9] i2c: i2c-mxs: replace platform_driver_probe to support deferred probing

2013-10-08 Thread Marek Vasut
Dear Wolfram Sang,

> Subsystems like pinctrl and gpio rightfully make use of deferred probing at
> core level. Now, deferred drivers won't be retried if they don't have a
> .probe function specified in the driver struct. Fix this driver to have
> that, so the devices it supports won't get lost in a deferred probe.
> 
> Signed-off-by: Wolfram Sang 
> Cc: Marek Vasut 

Certainly makes sense,

Acked-by: Marek Vasut 

Thanks!

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: linux-next: manual merge of the hid tree with Linus' tree

2012-09-09 Thread Marek Vasut
Dear Stephen Rothwell,

> Hi Jiri,
> 
> Today's linux-next merge of the hid tree got a conflict in
> drivers/hid/usbhid/hid-quirks.c between commit 67ddbb3e6568 ("HID: add
> NOGET quirk for Eaton Ellipse MAX UPS") from Linus' tree and commit
> 2843b673d034 ("HID: add quirk for Freescale i.MX28 ROM recovery") from
> the hid tree.
> 
> Just context changes.  I fixed it up (see below) and can carry the fix as
> necessary (no action is required).

Looks obviously correct, thank you Stephen!

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH for 3.7] mtd: nand: fix Samsung SLC NAND identification regression

2012-10-10 Thread Marek Vasut
Dear David Woodhouse,

> On Tue, 2012-10-09 at 23:39 -0700, Brian Norris wrote:
> > I can see if that's possible, but I think it's unlikely. They don't
> > even bother following standards (ONFI). Is this an obstacle to
> > merging?
> 
> No. I already pushed it.

Thanks guys!

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] backlight: lms283gf05: use devm_gpio_request_one

2012-10-17 Thread Marek Vasut
Dear Jingoo Han,

> By using devm_gpio_request_one it is possible to set the direction
> and initial value in one shot. Thus, using devm_gpio_request_one
> can make the code simpler.
> 
> Signed-off-by: Jingoo Han 
> Cc: Richard Purdie 
> Cc: Marek Vasut 

Acked-by: Marek Vasut 

> ---
>  drivers/video/backlight/lms283gf05.c |   10 +++---
>  1 files changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/video/backlight/lms283gf05.c
> b/drivers/video/backlight/lms283gf05.c index ea43f22..c452210 100644
> --- a/drivers/video/backlight/lms283gf05.c
> +++ b/drivers/video/backlight/lms283gf05.c
> @@ -158,13 +158,9 @@ static int __devinit lms283gf05_probe(struct
> spi_device *spi) int ret = 0;
> 
>   if (pdata != NULL) {
> - ret = devm_gpio_request(&spi->dev, pdata->reset_gpio,
> - "LMS285GF05 RESET");
> - if (ret)
> - return ret;
> -
> - ret = gpio_direction_output(pdata->reset_gpio,
> - !pdata->reset_inverted);
> + ret = devm_gpio_request_one(&spi->dev, pdata->reset_gpio,
> + GPIOF_DIR_OUT | !pdata->reset_inverted,
> +     "LMS285GF05 RESET");
>   if (ret)
>   return ret;
>   }

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] dma: add new DMA control commands

2012-10-18 Thread Marek Vasut
Dear Huang Shijie,

Why such massive CC ?

> 于 2012年10月18日 14:18, Vinod Koul 写道:
> > Why cant you do start (prepare clock etc) when you submit the descriptor
> > to dmaengine. Can be done in tx_submit callback.
> > Similarly remove the clock when dma transaction gets completed.
> 
> I ever thought this method too.
> 
> But it will become low efficient in the following case:
> 
>Assuming the gpmi-nand driver has to read out 1024 pages in one
> _SINGLE_ read operation.
> The gpmi-nand will submit the descriptor to dmaengine per page.

It will? Then GPMI NAND is flat our broken ... again.

> So with
> your method,
> the system will repeat the enable/disable dma clock 1024 time.

Yes, it is the driver that's wrong.

> At every
> enable/disable dma clock,
> the system has to enable the clock chain and it's parents ...
> 
> But with this patch, we only need to enable/disable dma clock one time,
> just at we select the nand chip.

You are fixing a driver problem at a framework level, wrong.

So, check how the MXS SPI driver handles descriptor chaining. Indeed, the 
Sigmatel screwed the DMA stuff good. But if you analyze the SPI driver, you'll 
notice a few important points that might come handy when you fix the GPMI NAND 
driver properly.

The direction to take here is:
1) Implement DMA chaining into the GPMI NAND driver
2) You might need to do one PIO transfer to reconfigure the IP registers 
between 
each segment of the DMA chain (just as MXS SPI does it)
3) You might run out of DMA descriptors when doing too long chains -- so you 
might need to fix that part of the mxs DMA driver.

> thanks
> Huang Shijie

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] dma: add new DMA control commands

2012-10-18 Thread Marek Vasut
Dear Huang Shijie,

> 于 2012年10月18日 15:14, Marek Vasut 写道:
> > Dear Huang Shijie,
> > 
> > Why such massive CC ?
> > 
> >> 于 2012年10月18日 14:18, Vinod Koul 写道:
> >>> Why cant you do start (prepare clock etc) when you submit the
> >>> descriptor to dmaengine. Can be done in tx_submit callback.
> >>> Similarly remove the clock when dma transaction gets completed.
> >> 
> >> I ever thought this method too.
> >> 
> >> But it will become low efficient in the following case:
> >> Assuming the gpmi-nand driver has to read out 1024 pages in one
> >> 
> >> _SINGLE_ read operation.
> >> The gpmi-nand will submit the descriptor to dmaengine per page.
> > 
> > It will? Then GPMI NAND is flat our broken ... again.
> 
> yes.
> 
> Please read the NAND chip spec about the comand READ PAGE(00h-30h) and
> the code
> nand_do_read_ops(). The nand chip limits us only read one page out one
> time. So the driver will submit the descriptor to dmaengine per page.

So we can't stream data from the chip? About time to adjust the MTD framework 
to 
allow that. Maybe implement a command queue?

> >> So with
> >> your method,
> >> the system will repeat the enable/disable dma clock 1024 time.
> > 
> > Yes, it is the driver that's wrong.
> 
> not the driver.
> 
> >> At every
> >> enable/disable dma clock,
> >> the system has to enable the clock chain and it's parents ...
> >> 
> >> But with this patch, we only need to enable/disable dma clock one time,
> >> just at we select the nand chip.
> > 
> > You are fixing a driver problem at a framework level, wrong.
> > 
> > So, check how the MXS SPI driver handles descriptor chaining. Indeed, the
> > Sigmatel screwed the DMA stuff good. But if you analyze the SPI driver,
> > you'll notice a few important points that might come handy when you fix
> > the GPMI NAND driver properly.
> > 
> > The direction to take here is:
> > 1) Implement DMA chaining into the GPMI NAND driver
> 
> How can i implement the DMA chain if the nand chip READ-PAGE command
> gives us the one page limit?

This smells like a time to extend the MTD api ?

> thanks
> Huang Shijie
> 
> > 2) You might need to do one PIO transfer to reconfigure the IP registers
> > between each segment of the DMA chain (just as MXS SPI does it)
> > 3) You might run out of DMA descriptors when doing too long chains -- so
> > you might need to fix that part of the mxs DMA driver.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] dma: add new DMA control commands

2012-10-18 Thread Marek Vasut
Dear Huang Shijie,

> 于 2012年10月18日 16:16, Marek Vasut 写道:
> > So we can't stream data from the chip? About time to adjust the MTD
> > framework to allow that. Maybe implement a command queue?
> 
> IMHO, it's not possible. Because the READ-PAGE(00h-30h) command needs to
> check the busy status
> which means we have to stop in the middle, so we can not chain the all
> the read-pages DMA commands.

Can the DMA not branch?

> thanks
> Huang Shijie

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] dma: add new DMA control commands

2012-10-18 Thread Marek Vasut
Dear Huang Shijie,

> 于 2012年10月18日 16:49, Marek Vasut 写道:
> > Dear Huang Shijie,
> > 
> >> 于 2012年10月18日 16:16, Marek Vasut 写道:
> >>> So we can't stream data from the chip? About time to adjust the MTD
> >>> framework to allow that. Maybe implement a command queue?
> >> 
> >> IMHO, it's not possible. Because the READ-PAGE(00h-30h) command needs to
> >> check the busy status
> >> which means we have to stop in the middle, so we can not chain the all
> >> the read-pages DMA commands.
> > 
> > Can the DMA not branch?
> 
> it's too complicated to the MTD layer, as well as the gpmi driver.

Can you please elaborate ?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Improve timer Resolution on imx28

2012-12-21 Thread Marek Vasut
Dear Torben Hohn,

> Hi,
> 
> this Series increases the timer frequency and decreases min_delta_ns.
> I saw pretty high max latencies with cyclictest: ~600us.
> 
> The cause was the min_delta_ns value of 0xf 32kHz clockcycles.
> But with the 32kHz Clock, the period is still around 30us.
> So we just increase the clock freqeuncy driving the timers
> here.

Thanks for the set. Just a really quick question -- won't this under no 
circumstance cause interrupt storm?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] gpio: ucb1400: Can be built as a module

2013-04-17 Thread Marek Vasut
Dear Jean Delvare,

> With the recent code cleanup from Marek Vasut, driver gpio-ucb1400 can
> be built as a module, so change symbol GPIO_UCB1400 from bool to
> tristate.
> 
> Signed-off-by: Jean Delvare 
> Cc: Marek Vasut 
> Cc: Grant Likely 
> Cc: Linus Walleij 

Reviewed-by: Marek Vasut 

Thanks

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] video: mxsfb: Fix colors display on lower color depth

2013-04-18 Thread Marek Vasut
Dear Maxime Ripard,

> The current code always registers as a 32 bits display, and uses the
> hardware to drop the MSB of each color to abjust to the interface width
> used by the panel.
> 
> This results on 18 bits (and probably 16 bits display as well) in colors
> being displayed poorly, because the MSB are obviously the most important
> bits for each color definition.
> 
> The default controller behaviour when using an interface width smaller
> than the color depth is to drop the LSBs of each color, which makes more
> sense because you lose the least important part of the color definition.
> 
> So, to fix the colors display, just get back to the default controller
> behaviour.
> 
> Signed-off-by: Maxime Ripard 

Isn't this what M28EVK is doing already? This was last tested on 3.9-rc3 I 
think:

bits-per-pixel = <16>;
bus-width = <18>;

But it's also true we have all 24 data lines routed to the LCD.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RESEND] video: mxsfb: Fix colors display on lower color depth

2013-04-22 Thread Marek Vasut
Dear Maxime Ripard,

> The current code always registers as a 32 bits display, and uses the
> hardware to drop the MSB of each color to abjust to the interface width
> used by the panel.
> 
> This results on 18 bits (and probably 16 bits display as well) in colors
> being displayed poorly, because the MSB are obviously the most important
> bits for each color definition.
> 
> The default controller behaviour when using an interface width smaller
> than the color depth is to drop the LSBs of each color, which makes more
> sense because you lose the least important part of the color definition.
> 
> So, to fix the colors display, just get back to the default controller
> behaviour.
> 
> Signed-off-by: Maxime Ripard 

Did you receive my latest email? Check M28EVK (imx28-m28evk.dts), it uses 18bit 
LCD and works without this patch I think.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RESEND] video: mxsfb: Fix colors display on lower color depth

2013-04-22 Thread Marek Vasut
Dear Maxime Ripard,

> Hi Marek,
> 
> Le 22/04/2013 11:16, Marek Vasut a écrit :
> > Dear Maxime Ripard,
> > 
> >> The current code always registers as a 32 bits display, and uses the
> >> hardware to drop the MSB of each color to abjust to the interface width
> >> used by the panel.
> >> 
> >> This results on 18 bits (and probably 16 bits display as well) in colors
> >> being displayed poorly, because the MSB are obviously the most important
> >> bits for each color definition.
> >> 
> >> The default controller behaviour when using an interface width smaller
> >> than the color depth is to drop the LSBs of each color, which makes more
> >> sense because you lose the least important part of the color definition.
> >> 
> >> So, to fix the colors display, just get back to the default controller
> >> behaviour.
> >> 
> >> Signed-off-by: Maxime Ripard 
> > 
> > Did you receive my latest email? Check M28EVK (imx28-m28evk.dts), it uses
> > 18bit LCD and works without this patch I think.
> 
> Thanks for the pointer. You mentionned in your other mail that it was
> wired on 24bits but that the screen is actually 18 bits. I went to
> search for the schematics to look at the wirings to see what could
> differ, but I couldn't find any for the M28EVK. Are they publicly
> available?

Yes, [1] page 6 .

[1] http://www.denx-cs.de/doku/sites/default/files/M28EVK_V20_0.pdf

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: How usable is spi-mxs driver?

2013-04-22 Thread Marek Vasut
Dear Hector Palacios,

> Hello,
> 
> I had the feeling that the MXS SPI driver was rather useless due to the SPI
> being only half-duplex on i.MX28, but it looks like people's using it so I
> was wondering what kind of SPI devices can be driven with it, or if there
> is any trick that allows bi-directional communication.

It can't do full duplex. I tested it with master-slave communication (half-
duplex) and various SPI flashes.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V3 5/5] input: pxa27x-keypad: add device tree support

2013-06-19 Thread Marek Vasut
Dear Chao Xie,

> Signed-off-by: Chao Xie 
> ---
>  .../devicetree/bindings/input/pxa27x-keypad.txt|   60 +
>  drivers/input/keyboard/pxa27x_keypad.c |  232
> +++- 2 files changed, 288 insertions(+), 4 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/input/pxa27x-keypad.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/pxa27x-keypad.txt
> b/Documentation/devicetree/bindings/input/pxa27x-keypad.txt new file mode
> 100644
> index 000..f8674f7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/pxa27x-keypad.txt
> @@ -0,0 +1,60 @@
> +* Marvell PXA Keypad controller
> +
> +Required Properties
> +- compatible : should be "marvell,pxa27x-keypad"
> +- reg : Address and length of the register set for the device
> +- interrupts : The interrupt for the keypad controller
> +- marvell,debounce-interval : How long time the key will be

Is there no generic prop name for this debounce interval?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv3 2/3] ARM: mxs: cfa10049: Switch bus i2c1 to bitbanging

2013-07-02 Thread Marek Vasut
Dear Alexandre Belloni,

> On 02/07/2013 13:50, Alexandre Belloni wrote:
> > On 02/07/2013 13:45, Fabio Estevam wrote:
> >> Shouldn't this be
> >> 
> >>  i2c@1 {
> >>  
> >>  reg = <1>; ?
> > 
> > No, we have 4 devices on that mux and 2 pins to select the muxing.
> 
> OK, got it working.
> 
> So, the results:
> 
> bitbanging:
> 
> # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw
> 2637
> real  0m 0.09s
> user  0m 0.01s
> sys   0m 0.01s
> 
> 
> i2c-mxs PIO mode:
> 
> # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw
> [   35.007650] [sched_delayed] sched: RT throttling activated
> 2627
> real  0m 7.14s
> user  0m 0.02s
> sys   0m 0.01s
> 
> 
> i2c-mxs PIO mode without LRADC:
> 
> # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw
> [   18.007432] [sched_delayed] sched: RT throttling activated
> 2629
> real  0m 7.09s
> user  0m 0.00s
> sys   0m 0.03s
> 
> 
> i2c-mxs DMA mode:
> 
> # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw
> 2631
> real  0m 0.12s
> user  0m 0.01s
> sys   0m 0.01s
> 
> 
> It seems fine for me.

I think I'm getting a little lost in these gazilions of i2c and lradc threads. 
Can we not create one thread and keep the related stuff in there instead of 
discussing it all around !?

Only one question comes to mind with this email -- what do LRADC and I2C have 
to 
do with each other here ?

It'd be nice if someone could summarize on what I should focus and possibly 
prepare a testcase.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: BUG: soft lockup when recording audio on MX28EVK with ASoC sgtl5000

2013-03-25 Thread Marek Vasut
; 8)
> [  936.356509] [] (mxs_timer_interrupt+0x1c/0x28) from
> [] (handle_irq_event_percpu+0x5c
> /0x26c)
> [  936.366647] [] (handle_irq_event_percpu+0x5c/0x26c) from
> [] (handle_irq_event+0x3c/0
> x5c)
> [  936.376519] [] (handle_irq_event+0x3c/0x5c) from []
> (handle_level_irq+0x8c/0x118)
> [  936.385781] [] (handle_level_irq+0x8c/0x118) from []
> (generic_handle_irq+0x28/0x30)
> [  936.395222] [] (generic_handle_irq+0x28/0x30) from
> [] (handle_IRQ+0x30/0x84)
> [  936.404050] [] (handle_IRQ+0x30/0x84) from []
> (icoll_handle_irq+0x30/0x44)
> [  936.412701] [] (icoll_handle_irq+0x30/0x44) from []
> (__irq_svc+0x44/0x54)
> [  936.421243] Exception stack(0xc6ed1d50 to 0xc6ed1d98)
> [  936.426320] 1d40: 46ec4000 c6ec91e0
> 2013 c6ec91e0
> [  936.434527] 1d60: c6ed c062e870  c743 0001 c6ec91e0
> c0455e0c c6ed1dc4
> [  936.442727] 1d80:  c6ed1d98 c004c9d4 c001a5c8 0013 
> [  936.449389] [] (__irq_svc+0x44/0x54) from []
> (cpu_arm926_switch_mm+0x8/0x20)
> [  936.458217] [] (cpu_arm926_switch_mm+0x8/0x20) from
> [] (0xc6e303c0)
> 
> The system does not completely die but it is extremely slow and almost
> unusable, I checked that the mxs-saif interrupt (184) counter suddenly
> increased very quickly (from 288 to 45427 in one second, 94471 the next
> second, and so on). I was wondering if this happens in other platforms
> using this codec or if it is an MXS issue only.
> 
> Best regards,

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/5] ARM: mxs-lradc: Simplify mxs_lradc_trigger_init error handle

2013-07-15 Thread Marek Vasut
Dear Otavio Salvador,

> As we have a 'ret' variable with the iio_trigger_register return, this
> can be used as result.
> 
> Signed-off-by: Otavio Salvador 
> ---
>  drivers/staging/iio/adc/mxs-lradc.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
> b/drivers/staging/iio/adc/mxs-lradc.c index d92c97a..6d41846 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -671,14 +671,12 @@ static int mxs_lradc_trigger_init(struct iio_dev
> *iio) trig->ops = &mxs_lradc_trigger_ops;
> 
>   ret = iio_trigger_register(trig);
> - if (ret) {
> + if (ret)
>   iio_trigger_free(trig);
> - return ret;
> - }
> 
>   iio->trig = trig;


You are still setting iio->trig , you dont want to do that in case of failure

> - return 0;
> +     return ret;
>  }
> 
>  static void mxs_lradc_trigger_remove(struct iio_dev *iio)

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/5] ARM: mxs-lradc: Simplify mxs_lradc_trigger_init error handle

2013-07-15 Thread Marek Vasut
Dear Otavio Salvador,

> On Mon, Jul 15, 2013 at 11:24 AM, Marek Vasut  wrote:
> > Dear Otavio Salvador,
> > 
> >> As we have a 'ret' variable with the iio_trigger_register return, this
> >> can be used as result.
> >> 
> >> Signed-off-by: Otavio Salvador 
> >> ---
> >> 
> >>  drivers/staging/iio/adc/mxs-lradc.c | 6 ++
> >>  1 file changed, 2 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
> >> b/drivers/staging/iio/adc/mxs-lradc.c index d92c97a..6d41846 100644
> >> --- a/drivers/staging/iio/adc/mxs-lradc.c
> >> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> >> @@ -671,14 +671,12 @@ static int mxs_lradc_trigger_init(struct iio_dev
> >> *iio) trig->ops = &mxs_lradc_trigger_ops;
> >> 
> >>   ret = iio_trigger_register(trig);
> >> 
> >> - if (ret) {
> >> + if (ret)
> >> 
> >>   iio_trigger_free(trig);
> >> 
> >> - return ret;
> >> - }
> >> 
> >>   iio->trig = trig;
> > 
> > You are still setting iio->trig , you dont want to do that in case of
> > failure
> 
> In this case it'd end being NULL I think, wouldn't it?

It won't, see the implementation in:

drivers/iio/industrialio-trigger.c

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 1/5] iio: mxs-lradc: change the realbits to 12

2013-07-19 Thread Marek Vasut
Dear Hector Palacios,

> The LRADC virtual channels have an 18 bit field to store the sum of up
> to 2^5 accumulated samples. The read_raw function however only operates
> over a single sample (12 bit resolution).
> In order to use this field for scaling operations, we need it to be the
> exact resolution value of the LRADC.
> Besides, the driver was using an 18 bit mask (LRADC_CH_VALUE_MASK) to
> report touch coordinates to userland. A 12 bit mask should be used instead
> or else the touch libraries will expect a coordinates range between 0
> and 0x3 (18 bits), instead of between 0 and 0xfff (12 bits).
> 
> Signed-off-by: Hector Palacios 

Acked-by: Marek Vasut 

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 2/5] ARM: dts: add reference voltage property for MXS LRADC

2013-07-19 Thread Marek Vasut
Dear Hector Palacios,

> Some LRADC channels have fixed pre-dividers so they can measure
> different voltages at full scale. The reference voltage allows to
> expose a scaling attribute through the IIO sysfs so that a user can
> compute the real voltage out of a measured sample value.
> 
> Signed-off-by: Hector Palacios 
> ---
>  Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt | 9
> - arch/arm/boot/dts/imx23.dtsi   
> | 4  arch/arm/boot/dts/imx28.dtsi|
> 4  3 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git
> a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
> b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt index
> 4688205..6ec485c 100644
> --- a/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
> +++ b/Documentation/devicetree/bindings/staging/iio/adc/mxs-lradc.txt
> @@ -1,9 +1,12 @@
>  * Freescale i.MX28 LRADC device driver
> 
>  Required properties:
> -- compatible: Should be "fsl,imx28-lradc"
> +- compatible: "fsl,imx28-lradc", "fsl,imx23-lradc"

This is a separate fix, but I think it's not that important to rework the whole 
patchset because of it.

Acked-by: Marek Vasut 

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 5/5] iio: mxs-lradc: add write_raw function to modify scale

2013-07-19 Thread Marek Vasut
Dear Hector Palacios,

> Added write_raw function to manipulate the optional divider_by_two
> through the scaling attribute out of the available scales.
> 
> Signed-off-by: Hector Palacios 
> ---
>  drivers/staging/iio/adc/mxs-lradc.c | 55
> - 1 file changed, 54 insertions(+), 1
> deletion(-)
> 
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
> b/drivers/staging/iio/adc/mxs-lradc.c index c929733..286cde2 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -144,6 +144,7 @@ struct mxs_lradc {
> 
>   uint32_tvref_mv[LRADC_MAX_TOTAL_CHANS];
>   unsigned intscale_avail[LRADC_MAX_TOTAL_CHANS][2][2];
> + unsigned intis_divided[LRADC_MAX_TOTAL_CHANS];

Why not use bitfield ? ;-)

>   /*
>* Touchscreen LRADC channels receives a private slot in the CTRL4
> @@ -202,6 +203,7 @@ struct mxs_lradc {
>  #define  LRADC_CTRL1_LRADC_IRQ_OFFSET0
> 
>  #define  LRADC_CTRL2 0x20
> +#define  LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET24
>  #define  LRADC_CTRL2_TEMPSENSE_PWD   (1 << 15)
> 
>  #define  LRADC_STATUS0x40
> @@ -310,7 +312,8 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>   break;
>   case IIO_CHAN_INFO_SCALE:
>   *val = lradc->vref_mv[chan->channel];
> - *val2 = chan->scan_type.realbits;
> + *val2 = chan->scan_type.realbits -
> + lradc->is_divided[chan->channel];
>   ret = IIO_VAL_FRACTIONAL_LOG2;
>   break;
>   default:
> @@ -323,6 +326,54 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>   return ret;
>  }
> 
> +static int mxs_lradc_write_raw(struct iio_dev *iio_dev,
> +const struct iio_chan_spec *chan,
> +int val, int val2, long m)
> +{
> + struct mxs_lradc *lradc = iio_priv(iio_dev);
> + int ret;
> +
> + ret = mutex_trylock(&lradc->lock);
> + if (!ret)
> + return -EBUSY;
> +
> + switch (m) {
> + case IIO_CHAN_INFO_SCALE:
> + ret = -EINVAL;
> + if (val == lradc->scale_avail[chan->channel][0][0] &&
> + val2 == lradc->scale_avail[chan->channel][0][1]) {
> + /* [0] -> divider by two disabled */

This comment is confusing, you use [0] in different dimensions of the array. So 
is the stuff below.

Still, how does this even work, can you show me and example how to set the 
divider from userland ? Sorry, I'm a bit confused with this 3D-business here, 
even if the comment in previous patch made it a bit clearer. Actually you can 
use some #define to specify if you're accessing div/2 or div/1 portion of the 
array to make it more readable.

Like ... scale_avail[chan->channel][LRADC_DIV_BY_2][LRADC_DECIMAL_PART] ... 
would by nice.

> + writel(1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET,
> +lradc->base + LRADC_CTRL2 + STMP_OFFSET_REG_CLR);
> + lradc->is_divided[chan->channel] = 0;
> + ret = 0;
> + } else if (val == lradc->scale_avail[chan->channel][1][0] &&
> +val2 == lradc->scale_avail[chan->channel][1][1]) {
> + /* [1] -> divider by two enabled */
> + writel(1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET,
> +lradc->base + LRADC_CTRL2 + STMP_OFFSET_REG_SET);
> + lradc->is_divided[chan->channel] = 1;
> + ret = 0;
> + }
> +
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> +
> + mutex_unlock(&lradc->lock);
> +
> + return ret;
> +}
> +
> +static int mxs_lradc_write_raw_get_fmt(struct iio_dev *iio_dev,
> +const struct iio_chan_spec *chan,
> +long m)
> +{
> + return IIO_VAL_INT_PLUS_NANO;
> +}
> +
>  static ssize_t mxs_lradc_show_scale_available_ch(struct device *dev,
>   struct device_attribute *attr,
>   char *buf,
> @@ -400,6 +451,8 @@ static const struct attribute_group
> mxs_lradc_attribute_group = { static const struct iio_info
> mxs_lradc_iio_info = {
>   .driver_module  = THIS_MODULE,
>   .read_raw   = mxs_lradc_read_raw,
> + .write_raw  = mxs_lr

Re: [PATCH v2 3/5] iio: mxs-lradc: add scale attribute to channels

2013-07-19 Thread Marek Vasut
O_CHAN_INFO_SCALE), \
>   .channel = (idx),   \
>   .scan_type = {  \
>   .sign = 'u',        \
> @@ -960,6 +977,12 @@ static int mxs_lradc_probe(struct platform_device
> *pdev) goto err_addr;
>   }
> 
> + /* Grab Vref array from DT */
> + ret = of_property_read_u32_array(node, "fsl,vref", lradc->vref_mv,
> +  LRADC_MAX_TOTAL_CHANS);
> + if (ret)
> + goto err_addr;
> +
>   platform_set_drvdata(pdev, iio);
> 
>   init_completion(&lradc->completion);

Otherwise:

Acked-by: Marek Vasut 


Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 5/5] iio: mxs-lradc: add write_raw function to modify scale

2013-07-19 Thread Marek Vasut
Dear Hector Palacios,

> Dear Marek,
> 
> On 07/19/2013 04:39 PM, Marek Vasut wrote:
> > Dear Hector Palacios,
> > 
> >> Added write_raw function to manipulate the optional divider_by_two
> >> through the scaling attribute out of the available scales.
> >> 
> >> Signed-off-by: Hector Palacios 
> >> ---
> >> 
> >>   drivers/staging/iio/adc/mxs-lradc.c | 55
> >> 
> >> - 1 file changed, 54 insertions(+),
> >> 1 deletion(-)
> >> 
> >> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
> >> b/drivers/staging/iio/adc/mxs-lradc.c index c929733..286cde2 100644
> >> --- a/drivers/staging/iio/adc/mxs-lradc.c
> >> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> >> @@ -144,6 +144,7 @@ struct mxs_lradc {
> >> 
> >>uint32_tvref_mv[LRADC_MAX_TOTAL_CHANS];
> >>unsigned intscale_avail[LRADC_MAX_TOTAL_CHANS][2][2];
> >> 
> >> +  unsigned intis_divided[LRADC_MAX_TOTAL_CHANS];
> > 
> > Why not use bitfield ? ;-)
> 
> This is used in some math below and an unsigned int looked more
> appropriate:
> 
>   case IIO_CHAN_INFO_SCALE:
>   *val = lradc->vref_mv[chan->channel];
>   *val2 = chan->scan_type.realbits -
>   lradc->is_divided[chan->channel];
>   ret = IIO_VAL_FRACTIONAL_LOG2;
>   break;

Oh ok.

> >>/*
> >>
> >> * Touchscreen LRADC channels receives a private slot in the CTRL4
> >> 
> >> @@ -202,6 +203,7 @@ struct mxs_lradc {
> >> 
> >>   #define  LRADC_CTRL1_LRADC_IRQ_OFFSET0
> >>   
> >>   #define  LRADC_CTRL2 0x20
> >> 
> >> +#define   LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET24
> >> 
> >>   #define  LRADC_CTRL2_TEMPSENSE_PWD   (1 << 15)
> >>   
> >>   #define  LRADC_STATUS0x40
> >> 
> >> @@ -310,7 +312,8 @@ static int mxs_lradc_read_raw(struct iio_dev
> >> *iio_dev,
> >> 
> >>break;
> >>
> >>case IIO_CHAN_INFO_SCALE:
> >>*val = lradc->vref_mv[chan->channel];
> >> 
> >> -  *val2 = chan->scan_type.realbits;
> >> +  *val2 = chan->scan_type.realbits -
> >> +  lradc->is_divided[chan->channel];
> >> 
> >>ret = IIO_VAL_FRACTIONAL_LOG2;
> >>break;
> >>
> >>default:
> >> @@ -323,6 +326,54 @@ static int mxs_lradc_read_raw(struct iio_dev
> >> *iio_dev,
> >> 
> >>return ret;
> >>   
> >>   }
> >> 
> >> +static int mxs_lradc_write_raw(struct iio_dev *iio_dev,
> >> + const struct iio_chan_spec *chan,
> >> + int val, int val2, long m)
> >> +{
> >> +  struct mxs_lradc *lradc = iio_priv(iio_dev);
> >> +  int ret;
> >> +
> >> +  ret = mutex_trylock(&lradc->lock);
> >> +  if (!ret)
> >> +  return -EBUSY;
> >> +
> >> +  switch (m) {
> >> +  case IIO_CHAN_INFO_SCALE:
> >> +  ret = -EINVAL;
> >> +  if (val == lradc->scale_avail[chan->channel][0][0] &&
> >> +  val2 == lradc->scale_avail[chan->channel][0][1]) {
> >> +  /* [0] -> divider by two disabled */
> > 
> > This comment is confusing, you use [0] in different dimensions of the
> > array. So is the stuff below.
> > 
> > Still, how does this even work, can you show me and example how to set
> > the divider from userland ? Sorry, I'm a bit confused with this
> > 3D-business here, even if the comment in previous patch made it a bit
> > clearer. Actually you can use some #define to specify if you're
> > accessing div/2 or div/1 portion of the array to make it more readable.
> > 
> > Like ... scale_avail[chan->channel][LRADC_DIV_BY_2][LRADC_DECIMAL_PART]
> > ... would by nice.
> 
> Agreed.
> 
> How it works:
> # cd /sys/devices/8000.apb/8004.apbx/8005.lradc/iio:device0
> 
> Here you have three entries per channel:
> in_voltageX_raw   -> the sample raw value
> in_voltageX_scale -> the scale to multiply the raw value to get the 
voltage
> in mV in_voltageX_scale_available -> lists the available scales

Re: [PATCH v2 3/5] iio: mxs-lradc: add scale attribute to channels

2013-07-19 Thread Marek Vasut
Dear Hector Palacios,

> Dear Marek,
> 
> On 07/19/2013 04:30 PM, Marek Vasut wrote:
> >> @@ -228,39 +230,12 @@ struct mxs_lradc {
> >> 
> >>   #define LRADC_RESOLUTION 12
> >>   #define LRADC_SINGLE_SAMPLE_MASK ((1 << LRADC_RESOLUTION) - 1)
> >> 
> >> -/*
> >> - * Raw I/O operations
> >> - */
> >> -static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
> >> +static int mxs_lradc_read_single(struct iio_dev *iio_dev,
> >> 
> >>const struct iio_chan_spec *chan,
> >>int *val, int *val2, long m)
> >>   
> >>   {
> >>   
> >>struct mxs_lradc *lradc = iio_priv(iio_dev);
> >>int ret;
> >> 
> >> -  unsigned long mask;
> >> -
> >> -  if (m != IIO_CHAN_INFO_RAW)
> >> -  return -EINVAL;
> >> -
> >> -  /* Check for invalid channel */
> >> -  if (chan->channel > LRADC_MAX_TOTAL_CHANS)
> >> -  return -EINVAL;
> > 
> > This was already resolved, so this patch won't apply I'm afraid.
> 
> You mean the 'unsigned long mask', right?  Yeah, I think I had resolved
> that one before submitting, but looks like I didn't.
> The other check is not resolved afaik. We agreed to remove it, but on a
> different patch.

I mean the other check, yeah. A patch removing that should be applied already.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 5/5] iio: mxs-lradc: add write_raw function to modify scale

2013-07-20 Thread Marek Vasut
Dear Jonathan Cameron,

> On 07/19/2013 05:22 PM, Hector Palacios wrote:
> > Dear Marek,
> > 
> > On 07/19/2013 06:17 PM, Marek Vasut wrote:
> >>> Here you have three entries per channel:
> >>> in_voltageX_raw-> the sample raw value
> >>> in_voltageX_scale-> the scale to multiply the raw value to get the
> >> 
> >> voltage
> >> 
> >>> in mV in_voltageX_scale_available -> lists the available scales of the
> >>> channel
> >>> 
> >>> For example for channel 0:
> >>> 
> >>> # cat in_voltage0_scale_available
> >>> 0.451660156 0.903320312(two scales available, first with divider by
> >>> two disabled, second with divider by two enabled)
> >>> 
> >>> # cat in_voltage0_scale
> >>> 0.451660156(divider by two is currently disabled)
> >>> 
> >>> # cat in_voltage0_raw(shows measured value)
> >>> 1000
> >>> 
> >>> If you multiply the value by the scale you get: 1000 * 0.451660156 =
> >>> 451.6 mV
> >>> 
> >>> # echo 0.903320312 > in_voltage0_scale(enables the divider by two)
> >> 
> >> Ok, so I have to remember this value : '0.903320312' in case I want to
> >> enable divide-by-two functionality?
> > 
> > No you don't. That's why there is a 'in_voltage_scale_available' that
> > lists the available values.
> > 
> >> H ... why would this interface not work:
> >> echo 2 > in_voltage0_scale
> >> 
> >> or
> >> 
> >> echo 1 > in_voltage0_scale
> > 
> > An easy thing like that is what I first submitted, but it was rejected
> > and I was told to use the scale_available descriptor instead, which is
> > the common interface the rest of drivers use.
> 
> This comes down to allowing us to have one generic predictable interface
> (which sometimes ends up looking uggly!).  The key thing is that if you
> are outputing using the _raw sysfs interfaces, the aim is to avoid doing
> nasty maths in kernel to get to 'standard' units such as mV.

OK, understood.

> Hence that scale attribute tells you what to apply to the value.  If you
> just wanted it to be 1 or 2 then the in_voltage0_raw value would have to
> be a long and nasty decimal.  Now we do have the option of
> in_voltage0_calibscale which would be applied internally to the value but
> it really isn't for this purpose (it's for devices with a 'trim' control)
> and you'd still have scale set to 0.903320312 or similar. Although they
> have meaning obviously, in this case 1 and 2 are little more than 'magic'
> numbers.
> 
> Note that when things are quite, I'm at least in theory working on a
> cleaner interface for these 'available' attributes that would also provide
> in kernel access for IIO consumers. Basically this will be another
> callback to get either the 'range' of pseudo continuous variables or the
> 'available' set for parameters like this.

Thanks for the explanation!

> Being lazy I'm happy to let someone else clean this corner up if they like?
> *looks hopeful*

Please don't look at me, I already am fully loaded with fixing my mess all 
around ;-/

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 3/5] iio: mxs-lradc: add scale attribute to channels

2013-07-22 Thread Marek Vasut
Dear Hector Palacios,

> Hi Marek,
> 
> On 07/19/2013 06:14 PM, Marek Vasut wrote:
> > Dear Hector Palacios,
> > 
> >> Dear Marek,
> >> 
> >> On 07/19/2013 04:30 PM, Marek Vasut wrote:
> >>>> @@ -228,39 +230,12 @@ struct mxs_lradc {
> >>>> 
> >>>>#define LRADC_RESOLUTION  12
> >>>>#define LRADC_SINGLE_SAMPLE_MASK  ((1 << 
> >>>> LRADC_RESOLUTION) 
- 1)
> >>>> 
> >>>> -/*
> >>>> - * Raw I/O operations
> >>>> - */
> >>>> -static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
> >>>> +static int mxs_lradc_read_single(struct iio_dev *iio_dev,
> >>>> 
> >>>>  const struct iio_chan_spec *chan,
> >>>>  int *val, int *val2, long m)
> >>>>
> >>>>{
> >>>>
> >>>>  struct mxs_lradc *lradc = iio_priv(iio_dev);
> >>>>  int ret;
> >>>> 
> >>>> -unsigned long mask;
> >>>> -
> >>>> -if (m != IIO_CHAN_INFO_RAW)
> >>>> -return -EINVAL;
> >>>> -
> >>>> -/* Check for invalid channel */
> >>>> -if (chan->channel > LRADC_MAX_TOTAL_CHANS)
> >>>> -return -EINVAL;
> >>> 
> >>> This was already resolved, so this patch won't apply I'm afraid.
> >> 
> >> You mean the 'unsigned long mask', right?  Yeah, I think I had resolved
> >> that one before submitting, but looks like I didn't.
> >> The other check is not resolved afaik. We agreed to remove it, but on a
> >> different patch.
> > 
> > I mean the other check, yeah. A patch removing that should be applied
> > already.
> 
> Where exactly? It's not fixed in Jonathan's fixes-togreg branch, at least.
> Did you fixed it?

I use linux-next [1], should be it.

http://git.kernel.org/cgit/linux/kernel/git/next/linux-
next.git/log/drivers/staging/iio/adc/mxs-lradc.c

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2 3/5] iio: mxs-lradc: add scale attribute to channels

2013-07-22 Thread Marek Vasut
Hi Hector,

> Hi Marek,
> 
> On 07/22/2013 09:42 AM, Marek Vasut wrote:
> > Dear Hector Palacios,
> > 
> >> Hi Marek,
> >> 
> >> On 07/19/2013 06:14 PM, Marek Vasut wrote:
> >>> Dear Hector Palacios,
> >>> 
> >>>> Dear Marek,
> >>>> 
> >>>> On 07/19/2013 04:30 PM, Marek Vasut wrote:
> >>>>>> @@ -228,39 +230,12 @@ struct mxs_lradc {
> >>>>>> 
> >>>>>> #define LRADC_RESOLUTION   12
> >>>>>> #define LRADC_SINGLE_SAMPLE_MASK   ((1 << LRADC_RESOLUTION)
> > 
> > - 1)
> > 
> >>>>>> -/*
> >>>>>> - * Raw I/O operations
> >>>>>> - */
> >>>>>> -static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
> >>>>>> +static int mxs_lradc_read_single(struct iio_dev *iio_dev,
> >>>>>> 
> >>>>>>const struct iio_chan_spec *chan,
> >>>>>>int *val, int *val2, long m)
> >>>>>> 
> >>>>>> {
> >>>>>> 
> >>>>>>struct mxs_lradc *lradc = iio_priv(iio_dev);
> >>>>>>int ret;
> >>>>>> 
> >>>>>> -  unsigned long mask;
> >>>>>> -
> >>>>>> -  if (m != IIO_CHAN_INFO_RAW)
> >>>>>> -  return -EINVAL;
> >>>>>> -
> >>>>>> -  /* Check for invalid channel */
> >>>>>> -  if (chan->channel > LRADC_MAX_TOTAL_CHANS)
> >>>>>> -  return -EINVAL;
> >>>>> 
> >>>>> This was already resolved, so this patch won't apply I'm afraid.
> >>>> 
> >>>> You mean the 'unsigned long mask', right?  Yeah, I think I had
> >>>> resolved that one before submitting, but looks like I didn't.
> >>>> The other check is not resolved afaik. We agreed to remove it, but on
> >>>> a different patch.
> >>> 
> >>> I mean the other check, yeah. A patch removing that should be applied
> >>> already.
> >> 
> >> Where exactly? It's not fixed in Jonathan's fixes-togreg branch, at
> >> least. Did you fixed it?
> > 
> > I use linux-next [1], should be it.
> > 
> > http://git.kernel.org/cgit/linux/kernel/git/next/linux-
> > next.git/log/drivers/staging/iio/adc/mxs-lradc.c
> 
> That is removing the unsigned long mask, but not the check for invalid
> channel. I'm taking care of the unsigned long mask but an additional patch
> is needed to remove the check for invalid channel.

Sorry for the confusion.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 5/5] iio: mxs-lradc: add write_raw function to modify scale

2013-07-22 Thread Marek Vasut
Dear Hector Palacios,

> Added write_raw function to manipulate the optional divider_by_two
> through the scaling attribute out of the available scales.
> 
> Signed-off-by: Hector Palacios 
> ---

Yep, looks nicer,

Reviewed-by: Marek Vasut 

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 4/5] iio: mxs-lradc: add scale_available file to channels

2013-07-22 Thread Marek Vasut
Dear Hector Palacios,

> Adds in_voltageX_scale_available file for every channel to read
> the different available scales.
> There are two scales per channel:
>  [0] = divider_by_two disabled (default)
>  [1] = divider_by_two enabled
> The scale is a struct made of integer and nano parts to build
> a long decimal number.
> 
> Signed-off-by: Hector Palacios 
> ---

[...]

> @@ -1008,6 +1098,26 @@ static int mxs_lradc_probe(struct platform_device
> *pdev) if (ret)
>   goto err_trig;
> 
> + /* Populate available ADC input ranges */
> + for (i = 0; i < LRADC_MAX_TOTAL_CHANS; i++) {
> + for (s = 0; s < ARRAY_SIZE(lradc->scale_avail[i]); s++) {
> + /*
> +  * [s=0] = optional divider by two disabled (default)
> +  * [s=1] = optional divider by two enabled
> +  *
> +  * The scale is calculated by doing:
> +  *   Vref >> (realbits - s)
> +  * which multiplies by two on the second component
> +  * of the array.
> +  */
> + scale_uv = ((u64)lradc->vref_mv[i] * 1) >>
> +(iio->channels[i].scan_type.realbits - s);

Thinking about this, this is basically

vref_mv[CHANNEL] * 100 000 000
scale_uv = 
  2^(12 - s)

Where s can be either 0 or 1.

Why do you multiply it by 1 I don't quite understand, but maybe it's 
fully obvious. It should be documented though.



> + lradc->scale_avail[i][s].nano =
> + do_div(scale_uv, 1) * 10;

Are we not loosing precission here?

> + lradc->scale_avail[i][s].integer = scale_uv;
> + }
> + }
> +
>   /* Configure the hardware. */
>   mxs_lradc_hw_init(lradc);

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 2/5] ARM: dts: add reference voltage property for MXS LRADC

2013-07-22 Thread Marek Vasut
Dear Lars-Peter Clausen,

> On 07/22/2013 04:04 PM, Hector Palacios wrote:
> > Some LRADC channels have fixed pre-dividers so they can measure
> > different voltages at full scale. The reference voltage allows to
> > expose a scaling attribute through the IIO sysfs so that a user can
> > compute the real voltage out of a measured sample value.
> 
> I've said before that I'm not convinced that this is the right way to
> implement this. And considering what Thomas said here
> http://www.mail-archive.com/devicetree-discuss@lists.ozlabs.org/msg36691.ht
> ml I guess I'm not alone with that opinion.
> 
> - Lars

He's talking about different versions of the IP block, this is the same IP 
block, just connected to different inputs.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] m25p80 / fast read

2013-08-12 Thread Marek Vasut
Dear Sascha Hauer,

> The following adds support for the mr25h10 chip to to the m25p80
> driver.
> 
> This driver currently has the problem that it unconditionally uses
> the fast_read command once it's enabled in Kconfig which of course
> leads to problems with multiboard kernels. This series adds a flag
> to the chip specific data to solve this. Also some unnecessary ifdefs
> are removed.
> 
> Sascha

Went through the whole series,

Acked-by: Marek Vasut 

Thanks!

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/4] mtd: m25p80: Pass flags through CAT25_INFO macro

2013-08-12 Thread Marek Vasut
Dear Sascha Hauer,

> The flags may have to be overwritten, so add them to the CAT25_INFO
> macro.
> 
> Signed-off-by: Sascha Hauer 
> ---
>  drivers/mtd/devices/m25p80.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 2f3d2a5..75d4391 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -694,13 +694,13 @@ struct flash_info {
>   .flags = (_flags),  \
>   })
> 
> -#define CAT25_INFO(_sector_size, _n_sectors, _page_size, _addr_width)
\
> +#define CAT25_INFO(_sector_size, _n_sectors, _page_size, _addr_width,
> _flags)   \ ((kernel_ulong_t)&(struct flash_info) {   
> 
\
>   .sector_size = (_sector_size),  \
>   .n_sectors = (_n_sectors),  \
>   .page_size = (_page_size),  \
>   .addr_width = (_addr_width),\
> - .flags = M25P_NO_ERASE, \
> + .flags = (_flags),  \
>   })
> 
>  /* NOTE: double check command sets and memory organization when you add
> @@ -732,7 +732,7 @@ static const struct spi_device_id m25p_ids[] = {
>   { "en25qh256", INFO(0x1c7019, 0, 64 * 1024, 512, 0) },
> 
>   /* Everspin */
> - { "mr25h256", CAT25_INFO(  32 * 1024, 1, 256, 2) },
> + { "mr25h256", CAT25_INFO(  32 * 1024, 1, 256, 2, M25P_NO_ERASE) },
> 
>   /* GigaDevice */
>   { "gd25q32", INFO(0xc84016, 0, 64 * 1024,  64, SECT_4K) },
> @@ -846,11 +846,11 @@ static const struct spi_device_id m25p_ids[] = {
>   { "w25q256", INFO(0xef4019, 0, 64 * 1024, 512, SECT_4K) },
> 
>   /* Catalyst / On Semiconductor -- non-JEDEC */
> - { "cat25c11", CAT25_INFO(  16, 8, 16, 1) },
> - { "cat25c03", CAT25_INFO(  32, 8, 16, 2) },
> - { "cat25c09", CAT25_INFO( 128, 8, 32, 2) },
> - { "cat25c17", CAT25_INFO( 256, 8, 32, 2) },
> - { "cat25128", CAT25_INFO(2048, 8, 64, 2) },
> + { "cat25c11", CAT25_INFO(  16, 8, 16, 1, M25P_NO_ERASE) },
> + { "cat25c03", CAT25_INFO(  32, 8, 16, 2, M25P_NO_ERASE) },
> + { "cat25c09", CAT25_INFO( 128, 8, 32, 2, M25P_NO_ERASE) },
> + { "cat25c17", CAT25_INFO( 256, 8, 32, 2, M25P_NO_ERASE) },
> + { "cat25128", CAT25_INFO(2048, 8, 64, 2, M25P_NO_ERASE) },
>   { },
>  };
>  MODULE_DEVICE_TABLE(spi, m25p_ids);

I can't say I like the growing macro magic (voodoo?) in this file, but your 
patch doesn't break anything and I don't feel like reworking this, so:

Acked-by: Marek Vasut 

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] mmc: mxs-mmc: Add broken-cd devicetree property

2013-04-06 Thread Marek Vasut
Dear Alexandre Pereira da Silva,

> On Sat, Apr 6, 2013 at 10:58 AM, Marc Kleine-Budde  
> wrote:
> > According to the documentation broken card detect is something different
> > 
> > than non-removable:
> > >   - broken-cd: There is no card detection available; polling must be
> > >   used. - non-removable: non-removable slot (like eMMC); assume always
> > >   present.
> > 
> > But mxs-mmc set MMC_CAP_NEEDS_POLL unconditionally.
> 
> I will work on a better way to fix this.

NEEDS_POLL looks nice and seems to fit this case.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC] ARM: dts: mxs: leave card detect out of common mmc pins config

2013-04-08 Thread Marek Vasut
Dear Shawn Guo,

> On Mon, Apr 08, 2013 at 03:58:05PM +0200, Hector Palacios wrote:
> > On 04/08/2013 02:48 PM, Shawn Guo wrote:
> > >On Mon, Apr 08, 2013 at 12:12:20PM +0200, Hector Palacios wrote:
> > >>MicroSD card sockets don't usually have card detect line. This pin
> > >>is actually not needed for the MMC to work and it is more of a
> > >>platform design decission to have it.
> > >>The card detect pin already has a configuration entry of its own:
> > >>'mmc0_cd_cfg' so we complete the iomux configuration here and let
> > >>platforms to include it or not depending on whether the card detect
> > >>line is routed to the SD socket.
> > >
> > >Sounds sensible.
> > >
> > >>Signed-off-by: Hector Palacios 
> > >>---
> > >>
> > >>Hello,
> > >>
> > >>All imx28 based platforms except 'bluegiga,apx4devkit' and
> > >>'schulercontrol,imx28-sps1', use 'mmc0_cd_cfg' in their mmc
> > >>configuration so please check whether this patch would break these
> > >>platforms.
> > >
> > >I just tested the patch on imx28-evk and card-detection still works.  So
> > >patches applied, thanks.
> > 
> > The EVK and most platforms will work because they are using
> > 'mmc0_cd_cfg' so actually this patch does not change anything on
> > them.
> > Platforms 'bluegiga,apx4devkit' and 'schulercontrol,imx28-sps1'
> > however are not referencing 'mmc0_cd_cfg' so after applying this
> > patch they will have unconfigured CD line and they may break.
> 
> Ah, yes.  I thought that any board that has CD support has to reference
> 'mmc0_cd_cfg'.  That's not necessarily true.
> 
> > The driver will call get_cd() upon probing, which returns the status of
> > the CD line. Please check these two platforms before applying.
> 
> Ok, let's wait for people owning the boards to confirm.

Maybe you want to use MMC_CAP_NEEDS_POLL as was noted by someone before on the 
olinuxino -- the slot is there, it's just the CD line that's missing.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC] ARM: dts: mxs: leave card detect out of common mmc pins config

2013-04-09 Thread Marek Vasut
Dear Hector Palacios,

> Dear Marek Vasut,
> 
> On 04/08/2013 06:28 PM, Marek Vasut wrote:
> > Dear Shawn Guo,
> > 
> >> On Mon, Apr 08, 2013 at 03:58:05PM +0200, Hector Palacios wrote:
> >>> On 04/08/2013 02:48 PM, Shawn Guo wrote:
> >>>> On Mon, Apr 08, 2013 at 12:12:20PM +0200, Hector Palacios wrote:
> >>>>> MicroSD card sockets don't usually have card detect line. This pin
> >>>>> is actually not needed for the MMC to work and it is more of a
> >>>>> platform design decission to have it.
> >>>>> The card detect pin already has a configuration entry of its own:
> >>>>> 'mmc0_cd_cfg' so we complete the iomux configuration here and let
> >>>>> platforms to include it or not depending on whether the card detect
> >>>>> line is routed to the SD socket.
> >>>> 
> >>>> Sounds sensible.
> >>>> 
> >>>>> Signed-off-by: Hector Palacios 
> >>>>> ---
> >>>>> 
> >>>>> Hello,
> >>>>> 
> >>>>> All imx28 based platforms except 'bluegiga,apx4devkit' and
> >>>>> 'schulercontrol,imx28-sps1', use 'mmc0_cd_cfg' in their mmc
> >>>>> configuration so please check whether this patch would break these
> >>>>> platforms.
> >>>> 
> >>>> I just tested the patch on imx28-evk and card-detection still works. 
> >>>> So patches applied, thanks.
> >>> 
> >>> The EVK and most platforms will work because they are using
> >>> 'mmc0_cd_cfg' so actually this patch does not change anything on
> >>> them.
> >>> Platforms 'bluegiga,apx4devkit' and 'schulercontrol,imx28-sps1'
> >>> however are not referencing 'mmc0_cd_cfg' so after applying this
> >>> patch they will have unconfigured CD line and they may break.
> >> 
> >> Ah, yes.  I thought that any board that has CD support has to reference
> >> 'mmc0_cd_cfg'.  That's not necessarily true.
> >> 
> >>> The driver will call get_cd() upon probing, which returns the status of
> >>> the CD line. Please check these two platforms before applying.
> >> 
> >> Ok, let's wait for people owning the boards to confirm.
> > 
> > Maybe you want to use MMC_CAP_NEEDS_POLL as was noted by someone before
> > on the olinuxino -- the slot is there, it's just the CD line that's
> > missing.
> 
> I'm not sure of what you mean. The mxs-mmc.c driver already sets the
> MMC_CAP_NEEDS_POLL flag by default in the probe() function. My platform
> does not even route the CD line because the microSD socket does not have
> it.
> So what I have done is modify the driver to parse the property
> 'non-removable' from the device tree in order to set the
> MMC_CAP_NONREMOVABLE flag:

Yes, I get it. I have two remarks still:
1) The card is removable (you can pull it out from olinuxino's slot)
2) Why is the NEEDS_POLL set by default ?
3) Does the NEEDS_POLL not solve the issue with missing CD line?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH RFC] ARM: dts: mxs: leave card detect out of common mmc pins config

2013-04-09 Thread Marek Vasut
Hi Hector,

> Dear Marek Vasut,
> 
> On 04/09/2013 10:15 AM, Marek Vasut wrote:
> > Dear Hector Palacios,
> > 
> >> Dear Marek Vasut,
> >> 
> >> On 04/08/2013 06:28 PM, Marek Vasut wrote:
> >>> Dear Shawn Guo,
> >>> 
> >>>> On Mon, Apr 08, 2013 at 03:58:05PM +0200, Hector Palacios wrote:
> >>>>> On 04/08/2013 02:48 PM, Shawn Guo wrote:
> >>>>>> On Mon, Apr 08, 2013 at 12:12:20PM +0200, Hector Palacios wrote:
> >>>>>>> MicroSD card sockets don't usually have card detect line. This pin
> >>>>>>> is actually not needed for the MMC to work and it is more of a
> >>>>>>> platform design decission to have it.
> >>>>>>> The card detect pin already has a configuration entry of its own:
> >>>>>>> 'mmc0_cd_cfg' so we complete the iomux configuration here and let
> >>>>>>> platforms to include it or not depending on whether the card detect
> >>>>>>> line is routed to the SD socket.
> >>>>>> 
> >>>>>> Sounds sensible.
> >>>>>> 
> >>>>>>> Signed-off-by: Hector Palacios 
> >>>>>>> ---
> >>>>>>> 
> >>>>>>> Hello,
> >>>>>>> 
> >>>>>>> All imx28 based platforms except 'bluegiga,apx4devkit' and
> >>>>>>> 'schulercontrol,imx28-sps1', use 'mmc0_cd_cfg' in their mmc
> >>>>>>> configuration so please check whether this patch would break these
> >>>>>>> platforms.
> >>>>>> 
> >>>>>> I just tested the patch on imx28-evk and card-detection still works.
> >>>>>> So patches applied, thanks.
> >>>>> 
> >>>>> The EVK and most platforms will work because they are using
> >>>>> 'mmc0_cd_cfg' so actually this patch does not change anything on
> >>>>> them.
> >>>>> Platforms 'bluegiga,apx4devkit' and 'schulercontrol,imx28-sps1'
> >>>>> however are not referencing 'mmc0_cd_cfg' so after applying this
> >>>>> patch they will have unconfigured CD line and they may break.
> >>>> 
> >>>> Ah, yes.  I thought that any board that has CD support has to
> >>>> reference 'mmc0_cd_cfg'.  That's not necessarily true.
> >>>> 
> >>>>> The driver will call get_cd() upon probing, which returns the status
> >>>>> of the CD line. Please check these two platforms before applying.
> >>>> 
> >>>> Ok, let's wait for people owning the boards to confirm.
> >>> 
> >>> Maybe you want to use MMC_CAP_NEEDS_POLL as was noted by someone before
> >>> on the olinuxino -- the slot is there, it's just the CD line that's
> >>> missing.
> >> 
> >> I'm not sure of what you mean. The mxs-mmc.c driver already sets the
> >> MMC_CAP_NEEDS_POLL flag by default in the probe() function. My platform
> >> does not even route the CD line because the microSD socket does not have
> >> it.
> >> So what I have done is modify the driver to parse the property
> >> 'non-removable' from the device tree in order to set the
> > 
> >> MMC_CAP_NONREMOVABLE flag:
> > Yes, I get it. I have two remarks still:
> > 1) The card is removable (you can pull it out from olinuxino's slot)
> 
> True, I misunderstood the use of 'non-removable'. So I guess I could use
> 'broken-cd' property instead, right?

Yep, seems ok.

> > 2) Why is the NEEDS_POLL set by default ?
> 
> Because the CD line cannot cause an interrupt in this controller.

Ok, understood.

NOTE: Here is an idea -- we can solve this the same way it's solved with the 
MXC 
SDHCI, configure the CD pin(s) as GPIO(s) and then they'll be able to generate 
interrupt. On the other hand, this shall definitelly be done in a separate 
patch.

> > 3) Does the NEEDS_POLL not solve the issue with missing CD line?
> 
> No. CD polling relies on the status register. The field CARD_DETECT in
> HW_SSP_STATUS register directly reflects the state of the SSP_DETECT input
> pin. If the pin is not connected it can have any value, so I guess we need
> a custom flag on the driver:

Understood!

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2] UCB1400: Pass ucb1400-gpio data through ac97 bus

2013-04-14 Thread Marek Vasut
Cc: Linus Walleij 
Cc: Jean Delvare 
Cc: Samuel Ortiz 
Cc: Mark Brown 
Cc: Guenter Roeck 
Cc: linux-kernel 
Cc: Grant Likely 
Signed-off-by: Marek Vasut 
---
 drivers/gpio/gpio-ucb1400.c |   19 ++-
 drivers/mfd/ucb1400_core.c  |5 +
 include/linux/ucb1400.h |   18 ++
 3 files changed, 17 insertions(+), 25 deletions(-)

v2: Rebase patch from:
http://lists.infradead.org/pipermail/linux-arm-kernel/2010-October/028656.html

NOTE: I didn't even compile-test this, but the fix was plenty straightforward.

diff --git a/drivers/gpio/gpio-ucb1400.c b/drivers/gpio/gpio-ucb1400.c
index 26405ef..6d0feb2 100644
--- a/drivers/gpio/gpio-ucb1400.c
+++ b/drivers/gpio/gpio-ucb1400.c
@@ -12,8 +12,6 @@
 #include 
 #include 
 
-struct ucb1400_gpio_data *ucbdata;
-
 static int ucb1400_gpio_dir_in(struct gpio_chip *gc, unsigned off)
 {
struct ucb1400_gpio *gpio;
@@ -50,7 +48,7 @@ static int ucb1400_gpio_probe(struct platform_device *dev)
struct ucb1400_gpio *ucb = dev->dev.platform_data;
int err = 0;
 
-   if (!(ucbdata && ucbdata->gpio_offset)) {
+   if (!(ucb && ucb->gpio_offset)) {
err = -EINVAL;
goto err;
}
@@ -58,7 +56,7 @@ static int ucb1400_gpio_probe(struct platform_device *dev)
platform_set_drvdata(dev, ucb);
 
ucb->gc.label = "ucb1400_gpio";
-   ucb->gc.base = ucbdata->gpio_offset;
+   ucb->gc.base = ucb->gpio_offset;
ucb->gc.ngpio = 10;
ucb->gc.owner = THIS_MODULE;
 
@@ -72,8 +70,8 @@ static int ucb1400_gpio_probe(struct platform_device *dev)
if (err)
goto err;
 
-   if (ucbdata && ucbdata->gpio_setup)
-   err = ucbdata->gpio_setup(&dev->dev, ucb->gc.ngpio);
+   if (ucb && ucb->gpio_setup)
+   err = ucb->gpio_setup(&dev->dev, ucb->gc.ngpio);
 
 err:
return err;
@@ -85,8 +83,8 @@ static int ucb1400_gpio_remove(struct platform_device *dev)
int err = 0;
struct ucb1400_gpio *ucb = platform_get_drvdata(dev);
 
-   if (ucbdata && ucbdata->gpio_teardown) {
-   err = ucbdata->gpio_teardown(&dev->dev, ucb->gc.ngpio);
+   if (ucb && ucb->gpio_teardown) {
+   err = ucb->gpio_teardown(&dev->dev, ucb->gc.ngpio);
if (err)
return err;
}
@@ -103,11 +101,6 @@ static struct platform_driver ucb1400_gpio_driver = {
},
 };
 
-void __init ucb1400_gpio_set_data(struct ucb1400_gpio_data *data)
-{
-   ucbdata = data;
-}
-
 module_platform_driver(ucb1400_gpio_driver);
 
 MODULE_DESCRIPTION("Philips UCB1400 GPIO driver");
diff --git a/drivers/mfd/ucb1400_core.c b/drivers/mfd/ucb1400_core.c
index daf6952..e9031fa 100644
--- a/drivers/mfd/ucb1400_core.c
+++ b/drivers/mfd/ucb1400_core.c
@@ -75,6 +75,11 @@ static int ucb1400_core_probe(struct device *dev)
 
/* GPIO */
ucb_gpio.ac97 = ac97;
+   if (pdata) {
+   ucb_gpio.gpio_setup = pdata->gpio_setup;
+   ucb_gpio.gpio_teardown = pdata->gpio_teardown;
+   ucb_gpio.gpio_offset = pdata->gpio_offset;
+   }
ucb->ucb1400_gpio = platform_device_alloc("ucb1400_gpio", -1);
if (!ucb->ucb1400_gpio) {
err = -ENOMEM;
diff --git a/include/linux/ucb1400.h b/include/linux/ucb1400.h
index d21b33c..2e9ee4d 100644
--- a/include/linux/ucb1400.h
+++ b/include/linux/ucb1400.h
@@ -83,15 +83,12 @@
 #define UCB_ID 0x7e
 #define UCB_ID_1400 0x4304
 
-struct ucb1400_gpio_data {
-   int gpio_offset;
-   int (*gpio_setup)(struct device *dev, int ngpio);
-   int (*gpio_teardown)(struct device *dev, int ngpio);
-};
-
 struct ucb1400_gpio {
struct gpio_chipgc;
struct snd_ac97 *ac97;
+   int gpio_offset;
+   int (*gpio_setup)(struct device *dev, int ngpio);
+   int (*gpio_teardown)(struct device *dev, int ngpio);
 };
 
 struct ucb1400_ts {
@@ -110,6 +107,9 @@ struct ucb1400 {
 
 struct ucb1400_pdata {
int irq;
+   int gpio_offset;
+   int (*gpio_setup)(struct device *dev, int ngpio);
+   int (*gpio_teardown)(struct device *dev, int ngpio);
 };
 
 static inline u16 ucb1400_reg_read(struct snd_ac97 *ac97, u16 reg)
@@ -162,10 +162,4 @@ static inline void ucb1400_adc_disable(struct snd_ac97 
*ac97)
 unsigned int ucb1400_adc_read(struct snd_ac97 *ac97, u16 adc_channel,
  int adcsync);
 
-#ifdef CONFIG_GPIO_UCB1400
-void __init ucb1400_gpio_set_data(struct ucb1400_gpio_data *data);
-#else
-static inline void ucb1400_gpio_set_data(struct ucb1400_gpio_data *data) {}
-#endif
-
 #endif
-- 
1.7.10.4

--
To unsubscribe from this list

Re: [PATCH v2] UCB1400: Pass ucb1400-gpio data through ac97 bus

2013-04-14 Thread Marek Vasut
Dear Marek Vasut,

> Cc: Linus Walleij 
> Cc: Jean Delvare 
> Cc: Samuel Ortiz 
> Cc: Mark Brown 
> Cc: Guenter Roeck 
> Cc: linux-kernel 
> Cc: Grant Likely 
> Signed-off-by: Marek Vasut 
> ---
>  drivers/gpio/gpio-ucb1400.c |   19 ++-
>  drivers/mfd/ucb1400_core.c  |5 +
>  include/linux/ucb1400.h |   18 ++
>  3 files changed, 17 insertions(+), 25 deletions(-)
> 
> v2: Rebase patch from:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2010-October/028656.h
> tml
> 
> NOTE: I didn't even compile-test this, but the fix was plenty
> straightforward.

But damn, this code is ugly. I'm retrospectively-ashamed.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: MXS USB otg device support

2013-05-28 Thread Marek Vasut
Dear Fabio Estevam,

> Hi Hector,
> 
> On 05/28/2013 07:27 AM, Hector Palacios wrote:
> > Hello,
> > 
> > Am I right in assuming that the MXS USB on-the-go port does not
> > currently support the device (gadget) functionality?
> > Anybody out there working on that?
> 
> I am not aware of the latest status about chipidea usb otg support for
> i.mx.

I gave up on tracking that mayhem.

> Please post this question in the linux-usb list and add Peter Chen, the
> folks from Pengutronix and Alexander Shishkin in Cc.

Good idea. Also please keep us in loop.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: gpio-ucb1400

2013-03-30 Thread Marek Vasut
Dear Guenter Roeck,

> On Fri, Mar 29, 2013 at 08:46:39PM +0100, Jean Delvare wrote:
> > Hi all,
> > 
> > In September 2009, a driver for the GPIO function of the UCB1400 chip
> > was added to the kernel tree. The probe function of this driver requires
> > ucbdata to be set. The only place where this happens is in function
> > ucb1400_gpio_set_data(). This function was never call, and still isn't.
> > So this is dead code for 3.5 years as far as the upstream kernel is
> > concerned.
> > 
> > To make things worse, this driver can't be built as a module, for no
> > good reason that I can see.
> > 
> > Marek, can you explain what was the point of submitting this driver that
> > nobody can use?
> > 
> > I would like either this driver to be fixed so that it can be used (and
> > that would IMHO start with dropping the ugly ucb1400_gpio_set_data hook
> > and global variable ucbdata), or this driver to be dropped from the
> > kernel tree. If the driver is kept, it should be adjusted so that it can
> > be built as a module.
> > 
> > If I overlooked something, please let me know.
> 
> Interestingly, the author made an attempt to fix that with [1]. It looks
> like the rest of that series was merged, but this patch wasn't, though I
> don't find any information about the reason.

It's been a while. Guenter, thanks for finding that link, but I suspect the 
patch is heavily obsolete by now.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: gpio-ucb1400

2013-03-31 Thread Marek Vasut
Dear Guenter Roeck,

> On Sat, Mar 30, 2013 at 08:20:44PM +0100, Marek Vasut wrote:
> > Dear Guenter Roeck,
> > 
> > > On Fri, Mar 29, 2013 at 08:46:39PM +0100, Jean Delvare wrote:
> > > > Hi all,
> > > > 
> > > > In September 2009, a driver for the GPIO function of the UCB1400 chip
> > > > was added to the kernel tree. The probe function of this driver
> > > > requires ucbdata to be set. The only place where this happens is in
> > > > function ucb1400_gpio_set_data(). This function was never call, and
> > > > still isn't. So this is dead code for 3.5 years as far as the
> > > > upstream kernel is concerned.
> > > > 
> > > > To make things worse, this driver can't be built as a module, for no
> > > > good reason that I can see.
> > > > 
> > > > Marek, can you explain what was the point of submitting this driver
> > > > that nobody can use?
> > > > 
> > > > I would like either this driver to be fixed so that it can be used
> > > > (and that would IMHO start with dropping the ugly
> > > > ucb1400_gpio_set_data hook and global variable ucbdata), or this
> > > > driver to be dropped from the kernel tree. If the driver is kept, it
> > > > should be adjusted so that it can be built as a module.
> > > > 
> > > > If I overlooked something, please let me know.
> > > 
> > > Interestingly, the author made an attempt to fix that with [1]. It
> > > looks like the rest of that series was merged, but this patch wasn't,
> > > though I don't find any information about the reason.
> > 
> > It's been a while. Guenter, thanks for finding that link, but I suspect
> > the patch is heavily obsolete by now.
> 
> Oh, it most definitely is, starting with the gpio driver name. Just wonder
> why it was never applied, and why no one seems to have noticed or cared.
> 
> Jean is absolutely right - it should get fixed, or the driver should be
> dropped if no one is using it anyway.

I think ARM/palmtc was using this.

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] phy/micrel: Add support for KSZ8031

2013-03-11 Thread Marek Vasut
Dear Hector Palacios,

> Micrel PHY KSZ8031 is similar to KSZ8021 and also requires the special
> initialization of "Operation Mode Strap Override" in reg 0x16
> introduced in 212ea99 (phy/micrel: Implement support for KSZ8021).
> 
> Signed-off-by: Hector Palacios 

I dont have any board with this one, yet I think this change is correct, so:

Reviewed-by: Marek Vasut 

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: SLAVE Side SPI kernel driver development

2012-08-24 Thread Marek Vasut
Dear Fabio Estevam,

> On Fri, Aug 24, 2012 at 11:57 AM, Bastelli Carlo (yahoo)

CCing SPI list

>  wrote:
> > Hello, I'm Carlo, I have a difficult task at work, my boss asked me to
> > develop
> > 
> >  a driver SPI slave side on embedded ARM processor running Linux.
> > 
> > Precisely linux occur on the SPI bus as a slave, not master as required
> > by the hierarchical structure of the current kernel. The new driver
> > will
> > 
> >  receive via interrupt and respond when selected by its chip select.

So a soft-spi ? It's gonna be slow as hell and unstable too.

> >  Can
> >  anyone suggest the best way to develop this driver?

Get a CPU which has SPI-slave capable IP, start from there. If you were to 
implement soft-spi this way, you'd have to use FIQ on ARM, to be able to handle 
stuff with reasonable latency ... check how the ps/2-gpio driver is done in 
Linux ... I tried it, it barely worked, you don't want to go down that road.

> >  Personally, I
> > 
> > have identified two main roads, the first if possible to exploit the
> > structure SPI master driver

Not happening. You'd have to split the parts from the struct spi_master into 
common and master-only parts. That's for starters.

> > with a few tricks that now I can not
> > define. The second most radical, write a driver for a UART like, but
> > managing the SPI device.
> > Thank you for any suggestions or examples.

First of all, the SPI slave mode is COMPLETELY different from the master mode. 
You need to react on incoming CS-low ASAP, which might turn out to be a problem 
on it's own. Then, even if you do, you do not know how long the transfer is 
going to be, so you need to allocate enough buffers as the transfer progresses 
onwards.

In case of your soft-spi, how do you plan to sample the CLK line to sync clock?

> Marek got spi slave running on a mx28. See his patch:
> http://www.spinics.net/lists/arm-kernel/msg190860.html

This was tested (and developed) on this board:
http://www.denx-cs.de/?q=M28

The userland code is really easy too, simply pump it into spidev or read from 
it. But all that is only possible because of the SPI IP in the mx28 is nice.

> Regards,
> 
> Fabio Estevam

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: SLAVE Side SPI kernel driver development

2012-08-25 Thread Marek Vasut
Dear Bastelli Carlo,

> In my application in fact the slave is connected to the master without
> other devices on the bus, a simple point to point, and receives a frame of
> known dimensions.

What hardware do you use? What CPU is it?

> I think I will be very busy to read post in the coming days  :)

Good luck with that.

> Thanks to all of you who provided suggestions.
> Carlo Bastelli
> 
> - Original Message -
> From: "Ned Forrester" 
> To: "Marek Vasut" 
> Cc: "Fabio Estevam" ;
> ; "Bastelli Carlo (yahoo)"
> ; ;
> 
> Sent: Friday, August 24, 2012 6:54 PM
> Subject: Re: SLAVE Side SPI kernel driver development
> 
> On 08/24/2012 11:38 AM, Marek Vasut wrote:
> > Dear Fabio Estevam,
> > 
> >> On Fri, Aug 24, 2012 at 11:57 AM, Bastelli Carlo (yahoo)
> > 
> > CCing SPI list
> > 
> >>  wrote:
> >>> Hello, I'm Carlo, I have a difficult task at work, my boss asked me to
> >>> develop
> >>> 
> >>>  a driver SPI slave side on embedded ARM processor running Linux.
> >>> 
> >>> Precisely linux occur on the SPI bus as a slave, not master as required
> >>> by the hierarchical structure of the current kernel. The new driver
> >>> will
> >>> 
> >>>  receive via interrupt and respond when selected by its chip select.
> > 
> > So a soft-spi ? It's gonna be slow as hell and unstable too.
> > 
> >>>  Can
> >>>  anyone suggest the best way to develop this driver?
> > 
> > Get a CPU which has SPI-slave capable IP, start from there. If you were
> > to implement soft-spi this way, you'd have to use FIQ on ARM, to be able
> > to handle
> > stuff with reasonable latency ... check how the ps/2-gpio driver is done
> > in
> > Linux ... I tried it, it barely worked, you don't want to go down that
> > road.
> > 
> >>>  Personally, I
> >>> 
> >>> have identified two main roads, the first if possible to exploit the
> >>> structure SPI master driver
> > 
> > Not happening. You'd have to split the parts from the struct spi_master
> > into
> > common and master-only parts. That's for starters.
> > 
> >>> with a few tricks that now I can not
> >>> define. The second most radical, write a driver for a UART like, but
> >>> managing the SPI device.
> >>> Thank you for any suggestions or examples.
> > 
> > First of all, the SPI slave mode is COMPLETELY different from the master
> > mode.
> > You need to react on incoming CS-low ASAP, which might turn out to be a
> > problem
> > on it's own. Then, even if you do, you do not know how long the transfer
> > is
> > going to be, so you need to allocate enough buffers as the transfer
> > progresses
> > onwards.
> > 
> > In case of your soft-spi, how do you plan to sample the CLK line to sync
> > clock?
> > 
> >> Marek got spi slave running on a mx28. See his patch:
> >> http://www.spinics.net/lists/arm-kernel/msg190860.html
> > 
> > This was tested (and developed) on this board:
> > http://www.denx-cs.de/?q=M28
> > 
> > The userland code is really easy too, simply pump it into spidev or read
> > from
> > it. But all that is only possible because of the SPI IP in the mx28 is
> > nice.
> > 
> >> Regards,
> >> 
> >> Fabio Estevam
> > 
> > Best regards,
> > Marek Vasut
> 
> I generally agree with the above comments.  A generalized SPI slave must
> be able to respond instantly, often within one clock cycle.  This can
> only work in hardware, or in an OS like Linux when the activity on the
> bus is completely predictable by the Linux slave.  The only cases I know
> of are streaming of data in a predictable way between a single master
> and single slave (no other devices on the bus).
> 
> This topic comes up periodically, but I don't think there has been any
> change in the situation for all the time I've followed the discussion.
> I recommend that you read the archives of this mailing list to
> understand the limitations of a potential slave mode.  The archives can
> be found at:
> 
> http://sourceforge.net/mailarchive/forum.php?forum_name=spi-devel-general
> 
> then look for threads with "slave" in the subject in the following
> months (I'd give thread start dates, but parts of the threads are
> disconnected in the archives, and this is the easiest way to see all the
> parts, day ranges are in parentheses):
> 
> 2007/04 (13-18)
> 2007/09/27 and 2007/10 (13-17)
> 2009/03 (2-3)
> 2009/12/18 and 2010/1/19 and 2010/02 (14-16)
> 2010/07 (26) and 2010/08 (13)
> 2011/01 (28-31) and 2011/02 (1-10)
> 2011/10 (18)
> 2012/06 (29-30 and 2012/07 (1-17)

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 4/4] drivers/mmc/host/mxs-mmc.c: fix error return code

2012-09-17 Thread Marek Vasut
Dear Peter Senna Tschudin,

> From: Peter Senna Tschudin 

The patch description is a bit crap, but otherwise

Acked-by: Marek Vasut 

> Convert a nonnegative error return code to a negative one, as returned
> elsewhere in the function.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // 
> (
> if@p1 (\(ret < 0\|ret != 0\))
>  { ... return ret; }
> 
> ret@p1 = 0
> )
> ... when != ret = e1
> when != &ret
> *if(...)
> {
>   ... when != ret = e2
>   when forall
>  return ret;
> }
> // 
> 
> Signed-off-by: Peter Senna Tschudin 
> ---
>  drivers/mmc/host/mxs-mmc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
> index 80d1e6d..afd8fdd 100644
> --- a/drivers/mmc/host/mxs-mmc.c
> +++ b/drivers/mmc/host/mxs-mmc.c
> @@ -674,6 +674,7 @@ static int mxs_mmc_probe(struct platform_device *pdev)
>   if (!ssp->dmach) {
>   dev_err(mmc_dev(host->mmc),
>   "%s: failed to request dma\n", __func__);
> + ret = -ENODEV;
>   goto out_clk_put;
>   }

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/1] [RFC] uartclk from serial_core exposed to sysfs

2012-08-14 Thread Marek Vasut
Dear Tomas Hlavacek,

> Support for read/modify of uartclk via sysfs added.
> It may prove useful with some no-name cards that
> has different oscillator speeds and no distinguishing
> PCI IDs to allow autodetection. It allows better integration
> with udev and/or init scripts.
> 
> Signed-off-by: Tomas Hlavacek 
> ---
>  drivers/tty/serial/serial_core.c |   54
> ++ drivers/tty/tty_io.c | 
>  17 
>  include/linux/tty.h  |2 ++
>  3 files changed, 73 insertions(+)
> 
> diff --git a/drivers/tty/serial/serial_core.c
> b/drivers/tty/serial/serial_core.c index a21dc8e..059b438 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -2293,6 +2293,46 @@ struct tty_driver *uart_console_device(struct
> console *co, int *index) return p->tty_driver;
>  }
> 
> +static ssize_t get_attr_uartclk(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + int ret;
> +
> + struct uart_state *state = (struct uart_state *)(dev_get_drvdata(dev));

You don't need this cast here.

> + mutex_lock(&state->port.mutex);
> + ret = snprintf(buf, PAGE_SIZE, "%d\n", state->uart_port->uartclk);

Do you really need such a large buffer (PAGE_SIZE) ?

> + mutex_unlock(&state->port.mutex);
> + return ret;
> +}
> +
> +static ssize_t set_attr_uartclk(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + struct uart_state *state = (struct uart_state *)(dev_get_drvdata(dev));
> + unsigned int val;
> + int ret;
> +
> + ret = kstrtouint(buf, 10, &val);
> + if (ret)
> + return ret;
> +
> + mutex_lock(&state->port.mutex);
> +
> + /*
> +  * Check value: baud_base has to be more than 9600
> +  * and uartclock = baud_base * 16 .
> +  */
> + if (val >= 153600)
> + state->uart_port->uartclk = val;

This magic value here would use some documentation.

> + mutex_unlock(&state->port.mutex);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(uartclk, S_IRUGO | S_IWUSR, get_attr_uartclk,
> + set_attr_uartclk);
> +
>  /**
>   *   uart_add_one_port - attach a driver-defined port structure
>   *   @drv: pointer to the uart low level driver structure for this port
> @@ -2355,6 +2395,14 @@ int uart_add_one_port(struct uart_driver *drv,
> struct uart_port *uport) }
> 
>   /*
> +  * Expose uartclk in sysfs. Use driverdata of the tty device for
> +  * referencing the UART port.
> +  */
> + dev_set_drvdata(tty_dev, state);
> + if (device_create_file(tty_dev, &dev_attr_uartclk) < 0)
> + dev_err(tty_dev, "Failed to add uartclk attr\n");
> +
> + /*
>* Ensure UPF_DEAD is not set.
>*/
>   uport->flags &= ~UPF_DEAD;
> @@ -2397,6 +2445,12 @@ int uart_remove_one_port(struct uart_driver *drv,
> struct uart_port *uport) mutex_unlock(&port->mutex);
> 
>   /*
> +  * Remove uartclk file from sysfs.
> +  */
> + device_remove_file(tty_lookup_device(drv->tty_driver, uport->line),
> + &dev_attr_uartclk);
> +
> + /*
>* Remove the devices from the tty layer
>*/
>   tty_unregister_device(drv->tty_driver, uport->line);
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index b425c79..8ea8622 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -3049,6 +3049,23 @@ void tty_unregister_device(struct tty_driver
> *driver, unsigned index) }
>  EXPORT_SYMBOL(tty_unregister_device);
> 
> +/*
> + *   tty_lookup_device - lookup a tty device
> + *   @driver: the tty driver that describes the tty device
> + *   @index: the index in the tty driver for this tty device
> + *
> + *   This function returns a struct device pointer when device has
> + *   been found and NULL otherwise.
> + *
> + *   Locking: ??
> + */
> +struct device *tty_lookup_device(struct tty_driver *driver, unsigned
> index) +{
> + dev_t devt = MKDEV(driver->major, driver->minor_start) + index;
> + return class_find_device(tty_class, NULL, &devt, dev_match_devt);
> +}
> +EXPORT_SYMBOL(tty_lookup_device);
> +
>  struct tty_driver *__alloc_tty_driver(int lines, struct module *owner)
>  {
>   struct tty_driver *driver;
> diff --git a/include/linux/tty.h b/include/linux/tty.h
> index 9f47ab5..5d408a1 100644
> --- a/include/linux/tty.h
> +++ b/include/linux/tty.h
> @@ -410,6 +410,8 @@ extern int tty_register_driver(struct tty_driver
> *driver); extern int tty_unregister_driver(struct tty_driver *driver);
>  extern struct device *tty_register_device(struct tty_driver *driver,
> unsigned index, struct device *dev);
> +extern struct device *tty_lookup_device(struct tty_driver *driver,
> + unsigned index);
>  extern void tty_unregister_device(struct tty_driver *driver, unsigned
> index); extern int tty_read_r

Re: [PATCH] mtd: kill MTD_NAND_VERIFY_WRITE

2012-08-15 Thread Marek Vasut
Dear Huang Shijie,

> 于 2012年08月15日 15:06, Shmulik Ladkani 写道:
> > Hi Huang,
> > 
> > On Tue, 14 Aug 2012 22:38:45 -0400 Huang Shijie  wrote:
> >> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> >> index 588e989..0ca7257 100644
> >> --- a/drivers/mtd/nand/Kconfig
> >> +++ b/drivers/mtd/nand/Kconfig
> >> @@ -22,15 +22,6 @@ menuconfig MTD_NAND
> >> 
> >>   if MTD_NAND
> >> 
> >> -config MTD_NAND_VERIFY_WRITE
> >> -  bool "Verify NAND page writes"
> >> -  help
> >> -This adds an extra check when data is written to the flash. The
> >> -NAND flash device internally checks only bits transitioning
> >> -from 1 to 0. There is a rare possibility that even though the
> >> -device thinks the write was successful, a bit could have been
> >> -flipped accidentally due to device wear or something else.
> >> -
> > 
> > There are some defconfig files which set CONFIG_MTD_NAND_VERIFY_WRITE.
> > 
> > I guess you should submit an accompanying patch that removes
> > CONFIG_MTD_NAND_VERIFY_WRITE from all defconfig files.
> 
> thanks a lot.
> 
> I will send out a separate patch to fix it.

I'd still prefer for this to be rather fixed. It seems to be able to find some 
obvious mistakes etc.

[...]

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] mtd: kill MTD_NAND_VERIFY_WRITE

2012-08-16 Thread Marek Vasut
Dear Huang Shijie,

> 于 2012年08月15日 20:09, Marek Vasut 写道:
> > I'd still prefer for this to be rather fixed. It seems to be able to find
> > some obvious mistakes etc.
> 
> could you please point out the mistakes?

Usual stuff -- flaws in drivers, misbehavior of the chip etc

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv3 1/1] [RFC] uartclk from serial_core exposed to sysfs

2012-08-17 Thread Marek Vasut
Dear Greg KH,

> On Fri, Aug 17, 2012 at 06:30:36PM +0200, Tomas Hlavacek wrote:
> > Hello Greg!
> > 
> > On Fri, Aug 17, 2012 at 5:06 PM, Greg KH  wrote:
> > >> @@ -2355,6 +2373,14 @@ int uart_add_one_port(struct uart_driver *drv,
> > >> struct uart_port *uport)
> > >> 
> > >>   }
> > >>   
> > >>   /*
> > >> 
> > >> +  * Expose uartclk in sysfs. Use driverdata of the tty device for
> > >> +  * referencing the UART port.
> > >> +  */
> > >> + dev_set_drvdata(tty_dev, port);
> > >> + if (device_create_file(tty_dev, &dev_attr_uartclk) < 0)
> > >> + dev_err(tty_dev, "Failed to add uartclk attr\n");
> > > 
> > > I think you just raced with userspace in creating the file after the
> > > device was announced to userspace.  Are you sure it's ok?
> > > 
> > > If not (hint, I don't think so), please make it a default attribute of
> > > the device, which will then cause the file to be created before it is
> > > announced to userspace.  It will also be less code as you don't have to
> > > clean it up by hand :)
> > 
> > Do you mean I should modify the tty_register_device() function not to
> > use device_create() but it should rather do the device initialization
> > on it's own.
> 
> No, not at all.
> 
> > And I should add add the attribute (via struct attribute_group) to
> > struct device in between device_initialize() and device_add() calls.
> > Did I get it right?
> 
> No, make this a driver attribute, that way when the device is
> registered, it adds the attribute automagically to the device that is
> bound to it.

(hint, DEVICE_ATTR), right ?

> Does that make sense?
> 
> greg k-h

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/3] IIO: arm: Add LRADC to i.MX28 dts

2012-08-19 Thread Marek Vasut
Dear Shawn Guo,

> On 17 August 2012 10:57, Marek Vasut  wrote:
> > Thanks ... still, is there some key for those tags? Or do you invent them
> > at random and then let people guess what's right? Some git grep on
> > Documentation directory gets me nothing.
> 
> There is no official document for this.

Hm, maybe such document can be introduced?

> But generally, each subsystem
> has a convention on the subject prefix, so that the output of git
> commands like git pull, git shortlog looks consistent on the patch
> subject, and more importantly people can easily know subsystem the
> patch touches.
> 
> The convention for patches touching arch/arm is "ARM: ".
> 
> Regards,
> Shawn

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/3] IIO: arm: Add LRADC to i.MX28 dts

2012-08-20 Thread Marek Vasut
Dear Jonathan Cameron,

> On 20/08/12 08:52, Shawn Guo wrote:
> > On Mon, Aug 20, 2012 at 08:29:23AM +0100, Jonathan Cameron wrote:
> >> Mostly these prefixes are an excuse for grumpy maintainers to moan
> >> at people :)
> > 
> > Yeah, I have to become grumpy when my comment gets ignored and I have
> > to make it right (for several times) to avoid my upstream moaning at me.
> 
> Not at all. My upstream has never moaned at me about it, but I still
> like to moan about it myself ;)

Your upstream is enjoyable to work with :)

> > Is it so hard?  You are not creating a new subsystem but just patching
> > an existing file.  A "git log" on the file simply shows you how to do it
> 
> Indeed. A file listing these would be add just one more piece of largely
> pointless horrendous documentation.

I was thinking more like a list of tags ... based on directories or something. 
But giving it a further thought, I see the trouble now.

> Perhaps if Marek feels particularly strongly about this, proposing an
> additional line to for the SubmittingPatches document to say get your
> title format by looking at the log for what you are patching will do
> the job? (I can't immediately spot anything equivalent in there...)

That might just be it ;-)

Thanks

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] mfd: da9052-core: Fix request_threaded_irq() parameter

2012-09-27 Thread Marek Vasut
Dear Fabio Estevam,

> From: Fabio Estevam 
> 
> On a mx53qsb dt-kernel the da9052-core driver fails to probe:
> 
> da9052 1-0048: DA9052 ADC IRQ failed ret=-22
> 
> In request_threaded_irq() the first parameter is missing the
> da9052->irq_base.
> 
> Fix it and avoid the error.
> 
> Also define 'DA9052_IRQF' for improving readability.
> 
> Signed-off-by: Fabio Estevam 

Reviewed-by: Marek Vasut 

> ---
>  drivers/mfd/da9052-core.c |   10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mfd/da9052-core.c b/drivers/mfd/da9052-core.c
> index a0a62b2..a3450d2 100644
> --- a/drivers/mfd/da9052-core.c
> +++ b/drivers/mfd/da9052-core.c
> @@ -33,6 +33,7 @@
>  #define DA9052_IRQ_MASK_POS_60x20
>  #define DA9052_IRQ_MASK_POS_70x40
>  #define DA9052_IRQ_MASK_POS_80x80
> +#define DA9052_IRQF  (IRQF_TRIGGER_LOW | IRQF_ONESHOT)
> 
>  static bool da9052_reg_readable(struct device *dev, unsigned int reg)
>  {
> @@ -788,16 +789,15 @@ int __devinit da9052_device_init(struct da9052
> *da9052, u8 chip_id) da9052->irq_base = pdata->irq_base;
> 
>   ret = regmap_add_irq_chip(da9052->regmap, da9052->chip_irq,
> -   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> -   da9052->irq_base, &da9052_regmap_irq_chip,
> -   &da9052->irq_data);
> +   DA9052_IRQF, da9052->irq_base,
> +   &da9052_regmap_irq_chip, &da9052->irq_data);
>   if (ret < 0)
>   goto regmap_err;
> 
>   da9052->irq_base = regmap_irq_chip_get_base(da9052->irq_data);
> 
> - ret = request_threaded_irq(DA9052_IRQ_ADC_EOM, NULL, da9052_auxadc_irq,
> -IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> + ret = request_threaded_irq(da9052->irq_base + DA9052_IRQ_ADC_EOM, NULL,
> +    da9052_auxadc_irq, DA9052_IRQF,
>  "adc irq", da9052);
>   if (ret != 0)
>   dev_err(da9052->dev, "DA9052 ADC IRQ failed ret=%d\n", ret);

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] mtd: spi-nor: don't build Cadence QuadSPI on non-ARM

2016-07-19 Thread Marek Vasut
On 07/19/2016 10:05 PM, Brian Norris wrote:
> On Tue, Jul 19, 2016 at 08:03:00AM +0200, Stefan Roese wrote:
>> On 18.07.2016 22:20, Brian Norris wrote:
>>> Hmm, does x86 not define readsl()/writesl()? I can never tell what
>>> accessors are supposed to be "standard" across architectures.
>>>
>>> Either we need to drop the COMPILE_TEST or maybe make it (!X86 &&
>>> COMPILE_TEST).
>>
>> iowrite32_rep() etc should work for x86 as well.
> 
> Looks like it might. I'm not sure the original submitter can retest
> right now (travel), so I'd probably rather just take the easy fix for
> now, and we can widen to COMPILE_TEST later if desired.

Isn't there a generic readsl() and writesl() implementation in
include/asm-generic/io.h ?

> If I could get an ack on something like this, I'll apply it soon:

This is fine, I am making a note to revisit this.

> ---8<---
> From: Brian Norris 
> Date: Tue, 19 Jul 2016 13:02:40 -0700
> Subject: [PATCH] mtd: spi-nor: don't build Cadence QuadSPI on non-ARM
> 
> This controller driver is used only on ARM but is mostly written
> portably so it can build on other arch'es. Unfortunately, at least x86
> doesn't provibe readsl()/writesl() accessors. We could possibly fix this
> issue in the future by using io{read,write}32_rep() instead, but let's
> just drop the architectures we aren't using for now.
> 
> Signed-off-by: Brian Norris 
> ---
>  drivers/mtd/spi-nor/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 1e6f037923d9..4a682ee0f632 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -40,7 +40,7 @@ config SPI_ATMEL_QUADSPI
>  
>  config SPI_CADENCE_QUADSPI
>   tristate "Cadence Quad SPI controller"
> - depends on OF && (ARM || COMPILE_TEST)
> + depends on OF && ARM
>   help
> Enable support for the Cadence Quad SPI Flash controller.
>  
> 


-- 
Best regards,
Marek Vasut


Re: [PATCH] mtd: spi-nor: don't build Cadence QuadSPI on non-ARM

2016-07-19 Thread Marek Vasut
On 07/20/2016 04:50 AM, Brian Norris wrote:
> On Wed, Jul 20, 2016 at 03:50:27AM +0200, Marek Vasut wrote:
>> On 07/19/2016 10:05 PM, Brian Norris wrote:
>>> On Tue, Jul 19, 2016 at 08:03:00AM +0200, Stefan Roese wrote:
>>>> On 18.07.2016 22:20, Brian Norris wrote:
>>>>> Hmm, does x86 not define readsl()/writesl()? I can never tell what
>>>>> accessors are supposed to be "standard" across architectures.
>>>>>
>>>>> Either we need to drop the COMPILE_TEST or maybe make it (!X86 &&
>>>>> COMPILE_TEST).
>>>>
>>>> iowrite32_rep() etc should work for x86 as well.
>>>
>>> Looks like it might. I'm not sure the original submitter can retest
>>> right now (travel), so I'd probably rather just take the easy fix for
>>> now, and we can widen to COMPILE_TEST later if desired.
>>
>> Isn't there a generic readsl() and writesl() implementation in
>> include/asm-generic/io.h ?
> 
> Yes, but somehow x86 has managed to avoid that. I guess it's optional
> for arch//include/asm/io.h to include ? At any
> rate, I double-checked myself by adding '#error "blah"' to
> include/asm-generic/io.h, and x86 still seemed to build fine (at least
> for the modules I was checking, like cadence-quadspi.o).

Yep, I just checked the same and it's not included from
arch/x86/include/asm/io.h for whatever reason. Maybe this needs to be
fixed on x86 level?

>>> If I could get an ack on something like this, I'll apply it soon:
>>
>> This is fine, I am making a note to revisit this.
> 
> Cool. In that case...
> 
>>> ---8<---
>>> From: Brian Norris 
>>> Date: Tue, 19 Jul 2016 13:02:40 -0700
>>> Subject: [PATCH] mtd: spi-nor: don't build Cadence QuadSPI on non-ARM
>>>
>>> This controller driver is used only on ARM but is mostly written
>>> portably so it can build on other arch'es. Unfortunately, at least x86
>>> doesn't provibe readsl()/writesl() accessors. We could possibly fix this
>>> issue in the future by using io{read,write}32_rep() instead, but let's
>>> just drop the architectures we aren't using for now.
>>>
>>> Signed-off-by: Brian Norris 
> 
> Applied.

Thanks

> Brian
> 


-- 
Best regards,
Marek Vasut


Re: [PATCH] mtd: spi-nor: don't build Cadence QuadSPI on non-ARM

2016-07-19 Thread Marek Vasut
On 07/20/2016 05:25 AM, Brian Norris wrote:
> On Wed, Jul 20, 2016 at 04:58:08AM +0200, Marek Vasut wrote:
>> On 07/20/2016 04:50 AM, Brian Norris wrote:
>>> On Wed, Jul 20, 2016 at 03:50:27AM +0200, Marek Vasut wrote:
>>>> On 07/19/2016 10:05 PM, Brian Norris wrote:
>>>>> On Tue, Jul 19, 2016 at 08:03:00AM +0200, Stefan Roese wrote:
>>>>>> On 18.07.2016 22:20, Brian Norris wrote:
>>>>>>> Hmm, does x86 not define readsl()/writesl()? I can never tell what
>>>>>>> accessors are supposed to be "standard" across architectures.
>>>>>>>
>>>>>>> Either we need to drop the COMPILE_TEST or maybe make it (!X86 &&
>>>>>>> COMPILE_TEST).
>>>>>>
>>>>>> iowrite32_rep() etc should work for x86 as well.
>>>>>
>>>>> Looks like it might. I'm not sure the original submitter can retest
>>>>> right now (travel), so I'd probably rather just take the easy fix for
>>>>> now, and we can widen to COMPILE_TEST later if desired.
>>>>
>>>> Isn't there a generic readsl() and writesl() implementation in
>>>> include/asm-generic/io.h ?
>>>
>>> Yes, but somehow x86 has managed to avoid that. I guess it's optional
>>> for arch//include/asm/io.h to include ? At any
>>> rate, I double-checked myself by adding '#error "blah"' to
>>> include/asm-generic/io.h, and x86 still seemed to build fine (at least
>>> for the modules I was checking, like cadence-quadspi.o).
>>
>> Yep, I just checked the same and it's not included from
>> arch/x86/include/asm/io.h for whatever reason. Maybe this needs to be
>> fixed on x86 level?
> 
> Maybe. That's why I added the x86 maintainers. Maybe they'd respond
> better^Wmore loudly if I just sent a patch to do that :)
> 
> But seriously, doing the above really breaks things, even if I stick the
> include at the end of asm/io.h. There's plenty of stuff that the
> asm-generic version includes based on #ifndef some_accessor, except x86
> uses a static inline for their definition. So it seems it's not trivial
> to get an architecture to fall back gracefully to asm-generic; you have
> to put in some work. It also may not be all that desirable to have some
> allegedly generic version generate something that may not be safe on a
> given architecture (and I don't purport to understand the x86 memory
> model).
> 
> Additionally, it looks like asm-generic/io.h is actually only included
> in 14 of 33 arch'es, so it seems like that's really not a designated
> goal. It does make it awfully difficult to figure out what I/O accessors
> are *actually* portable though...

Ouch :-( Maybe this is an opportunity for cleanup then ?

I think disabling the compile test for now is good, but we should
revisit this once I'm back and capable of digging in properly. Thus
far, I am mostly handling my mails on the bullet trains (which are
awesome here in Japan, I tell you that ;-) ), so I'm really not able
to give this as much attention as it requires.

> Brian
> 


-- 
Best regards,
Marek Vasut


Re: [PATCH v4 0/6] K2G: Add QSPI support

2017-10-16 Thread Marek Vasut
On 10/16/2017 08:15 AM, Vignesh R wrote:
> 
> 
> On Tuesday 03 October 2017 10:49 AM, Vignesh R wrote:
>>
>>
>> This series adds support for Cadence QSPI IP present in TI's 66AK2G SoC.
>> The patches enhance the existing cadence-quadspi driver to support
>> loopback clock circuit, pm_runtime support and tweaks for 66AK2G SoC.
> 
> Gentle ping on the series...

It's IMO OK, Cyrille, can you pick it ?

>>
>> Change log:
>>
>> v4:
>> * New patch to fix error handling sequence in probe.
>>
>> v3:
>> * Fix build warnings reported by kbuild test bot.
>>
>> Resend:
>> * Rebase to latest linux-next.
>> * Collect Acked-bys
>>
>> v2:
>> * Drop DT patches. Will be sent as separate series as requested by
>>  maintainer.
>> * Split binding docs into separate patches.
>> * Address comments by Rob Herring.
>>
>> Vignesh R (6):
>>   mtd: spi-nor: cadence-quadspi: Add TI 66AK2G SoC specific compatible
>>   mtd: spi-nor: cadence-quadspi: add a delay in write sequence
>>   mtd: spi-nor: cadence-quadspi: Add new binding to enable loop-back
>> circuit
>>   mtd: spi-nor: cadence-quadspi: Add support to enable loop-back clock
>> circuit
>>   mtd: spi-nor: cadence-quadspi: Fix error path in probe
>>   mtd: spi-nor: cadence-quadspi: Add runtime PM support
>>
>>  .../devicetree/bindings/mtd/cadence-quadspi.txt|  7 ++-
>>  drivers/mtd/spi-nor/cadence-quadspi.c  | 55 
>> +++---
>>  2 files changed, 55 insertions(+), 7 deletions(-)
>>
> 


-- 
Best regards,
Marek Vasut


Re: [PATCH] usb: dwc2: disable erroneous overcurrent condition

2017-10-16 Thread Marek Vasut
On 10/16/2017 03:57 PM, Dinh Nguyen wrote:
> For the case where an external VBUS is used, we should enable the external
> VBUS comparator in the driver. This would prevent an unnecessary
> overcurrent error which would then disable the host port.
> 
> This patch uses the standard 'disable-over-current' binding to allow of the
> option of disabling the over-current condition.
> 
> Signed-off-by: Dinh Nguyen 

Reviewed-by: Marek Vasut 

Similar patch was in U-Boot for two years now and it does the trick indeed.

> ---
>  drivers/usb/dwc2/core.h   | 4 
>  drivers/usb/dwc2/hcd.c| 5 +
>  drivers/usb/dwc2/params.c | 3 +++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 8367d4f9..730d7eb 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -395,6 +395,9 @@ enum dwc2_ep0_state {
>   *   (default when phy_type is UTMI+ or ULPI)
>   *   1 - 6 MHz
>   *   (default when phy_type is Full Speed)
> + * @oc_disable:  Flag to disable overcurrent condition.
> + *   0 - Allow overcurrent condition to get detected
> + *   1 - Disable overcurrent condtion to get detected
>   * @ts_dline:   Enable Term Select Dline pulsing
>   *   0 - No (default)
>   *   1 - Yes
> @@ -492,6 +495,7 @@ struct dwc2_core_params {
>   bool dma_desc_fs_enable;
>   bool host_support_fs_ls_low_power;
>   bool host_ls_low_power_phy_clk;
> + bool oc_disable;
>  
>   u8 host_channels;
>   u16 host_rx_fifo_size;
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index c263114..5e20336 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -213,6 +213,11 @@ static int dwc2_hs_phy_init(struct dwc2_hsotg *hsotg, 
> bool select_phy)
>   usbcfg &= ~(GUSBCFG_PHYIF16 | GUSBCFG_DDRSEL);
>   if (hsotg->params.phy_ulpi_ddr)
>   usbcfg |= GUSBCFG_DDRSEL;
> +
> + /* Set external VBUS indicator as needed. */
> + if (hsotg->params.oc_disable)
> + usbcfg |= (GUSBCFG_ULPI_INT_VBUS_IND |
> +GUSBCFG_INDICATORPASSTHROUGH);
>   break;
>   case DWC2_PHY_TYPE_PARAM_UTMI:
>   /* UTMI+ interface */
> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> index a3ffe97..39e02cd 100644
> --- a/drivers/usb/dwc2/params.c
> +++ b/drivers/usb/dwc2/params.c
> @@ -335,6 +335,9 @@ static void dwc2_get_device_properties(struct dwc2_hsotg 
> *hsotg)
>  num);
>   }
>   }
> +
> +     if (of_find_property(hsotg->dev->of_node, "disable-over-current", NULL))
> + p->oc_disable = true;
>  }
>  
>  static void dwc2_check_param_otg_cap(struct dwc2_hsotg *hsotg)
> 


-- 
Best regards,
Marek Vasut


Re: [PATCH v4 0/6] K2G: Add QSPI support

2017-10-16 Thread Marek Vasut
On 10/17/2017 12:29 AM, Cyrille Pitchen wrote:
> Hi all,
> 
> Le 16/10/2017 à 11:28, Marek Vasut a écrit :
>> On 10/16/2017 08:15 AM, Vignesh R wrote:
>>>
>>>
>>> On Tuesday 03 October 2017 10:49 AM, Vignesh R wrote:
>>>>
>>>>
>>>> This series adds support for Cadence QSPI IP present in TI's 66AK2G SoC.
>>>> The patches enhance the existing cadence-quadspi driver to support
>>>> loopback clock circuit, pm_runtime support and tweaks for 66AK2G SoC.
>>>
>>> Gentle ping on the series...
>>
>> It's IMO OK, Cyrille, can you pick it ?
>>
> 
> The series looks good to me too.
> 
> Marek, could you please add you "Acked-by" or "Reviewed-by" tag for the
> record in patchwork ? It will be automatically added when I use
> "pwclient git-am -s" to merge the patches into the spi-nor/next branch.

Acked-by: Marek Vasut 

-- 
Best regards,
Marek Vasut


Re: [PATCH] regulator: bd9571mwv: Statize local symbols

2017-05-28 Thread Marek Vasut
On 05/28/2017 08:02 AM, Axel Lin wrote:
> These functions are only used by this driver, make them static.
> 
> Signed-off-by: Axel Lin 

Obviously correct,
Acked-by: Marek Vasut 

> ---
>  drivers/regulator/bd9571mwv-regulator.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/regulator/bd9571mwv-regulator.c 
> b/drivers/regulator/bd9571mwv-regulator.c
> index 8ba206f..c67a83d 100644
> --- a/drivers/regulator/bd9571mwv-regulator.c
> +++ b/drivers/regulator/bd9571mwv-regulator.c
> @@ -43,7 +43,7 @@ enum bd9571mwv_regulators { VD09, VD18, VD25, VD33, DVFS };
>   .linear_min_sel = _lmin,\
>   }
>  
> -int bd9571mwv_avs_get_moni_state(struct regulator_dev *rdev)
> +static int bd9571mwv_avs_get_moni_state(struct regulator_dev *rdev)
>  {
>   unsigned int val;
>   int ret;
> @@ -55,8 +55,8 @@ int bd9571mwv_avs_get_moni_state(struct regulator_dev *rdev)
>   return val & BD9571MWV_AVS_SET_MONI_MASK;
>  }
>  
> -int bd9571mwv_avs_set_voltage_sel_regmap(struct regulator_dev *rdev,
> -  unsigned int sel)
> +static int bd9571mwv_avs_set_voltage_sel_regmap(struct regulator_dev *rdev,
> + unsigned int sel)
>  {
>   int ret;
>  
> @@ -68,7 +68,7 @@ int bd9571mwv_avs_set_voltage_sel_regmap(struct 
> regulator_dev *rdev,
>rdev->desc->vsel_mask, sel);
>  }
>  
> -int bd9571mwv_avs_get_voltage_sel_regmap(struct regulator_dev *rdev)
> +static int bd9571mwv_avs_get_voltage_sel_regmap(struct regulator_dev *rdev)
>  {
>   unsigned int val;
>   int ret;
> @@ -87,8 +87,8 @@ int bd9571mwv_avs_get_voltage_sel_regmap(struct 
> regulator_dev *rdev)
>   return val;
>  }
>  
> -int bd9571mwv_reg_set_voltage_sel_regmap(struct regulator_dev *rdev,
> -  unsigned int sel)
> +static int bd9571mwv_reg_set_voltage_sel_regmap(struct regulator_dev *rdev,
> + unsigned int sel)
>  {
>   return regmap_write_bits(rdev->regmap, BD9571MWV_DVFS_SETVID,
>rdev->desc->vsel_mask, sel);
> 


-- 
Best regards,
Marek Vasut


Re: [PATCH v1 1/1] mtd: intel-spi: Add support for Intel Lewisburg SPI serial flash controller

2017-10-29 Thread Marek Vasut
On 10/29/2017 10:57 AM, sathyanarayanan.kuppusw...@linux.intel.com wrote:
> From: Kuppuswamy Sathyanarayanan 
> 
> This patch adds PCI IDs for Intel Lewisburg PCH SPI serial flash controller.
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan 
> 

Culling the CC list and adding relevant people.
Patch looks IMO OK, but I'd like RB from Mika.

> ---
>  drivers/mtd/spi-nor/intel-spi-pci.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/intel-spi-pci.c 
> b/drivers/mtd/spi-nor/intel-spi-pci.c
> index e826523..048c007 100644
> --- a/drivers/mtd/spi-nor/intel-spi-pci.c
> +++ b/drivers/mtd/spi-nor/intel-spi-pci.c
> @@ -64,6 +64,8 @@ static void intel_spi_pci_remove(struct pci_dev *pdev)
>  
>  static const struct pci_device_id intel_spi_pci_ids[] = {
>   { PCI_VDEVICE(INTEL, 0x19e0), (unsigned long)&bxt_info },
> + { PCI_VDEVICE(INTEL, 0xa1a4), (unsigned long)&bxt_info },
> + { PCI_VDEVICE(INTEL, 0xa224), (unsigned long)&bxt_info },
>   { },
>  };
>  MODULE_DEVICE_TABLE(pci, intel_spi_pci_ids);
> 


-- 
Best regards,
Marek Vasut


Re: [PATCH v1 1/1] mtd: intel-spi: Add support for Intel Lewisburg SPI serial flash controller

2017-10-29 Thread Marek Vasut
On 10/29/2017 11:16 AM, Mika Westerberg wrote:
> On Sun, Oct 29, 2017 at 11:09:50AM +0100, Marek Vasut wrote:
>> On 10/29/2017 10:57 AM, sathyanarayanan.kuppusw...@linux.intel.com wrote:
>>> From: Kuppuswamy Sathyanarayanan 
>>> 
>>>
>>> This patch adds PCI IDs for Intel Lewisburg PCH SPI serial flash controller.
>>>
>>> Signed-off-by: Kuppuswamy Sathyanarayanan 
>>> 
>>
>> Culling the CC list and adding relevant people.
>> Patch looks IMO OK, but I'd like RB from Mika.
> 
> There is already a patch for Lewisburg queued for v4.15 here:
> 
> http://git.infradead.org/l2-mtd.git/commitdiff/d92b0f18a2039ff736b4296ad3cf3d505512051e

Heh, even better. Why does this patch add another ID ?

-- 
Best regards,
Marek Vasut


Re: [PATCH] iio: mxs-lradc: Fix temperature offset

2015-10-06 Thread Marek Vasut

On 10/06/2015 16:22, Alexandre Belloni wrote:

0° Kelvin is actually −273.15°C, not -272.15°C. Fix the temperature offset.

Reported-by: Janusz Użycki 
Signed-off-by: Alexandre Belloni 


Nice find :-)

Acked-by: Marek Vasut 

(I am using different MUA, please pardon the possible issues)


---
  drivers/staging/iio/adc/mxs-lradc.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c 
b/drivers/staging/iio/adc/mxs-lradc.c
index 3f7715c9968b..67bfa54c4fb1 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -916,10 +916,10 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
if (chan->type == IIO_TEMP) {
/* The calculated value from the ADC is in Kelvin, we
 * want Celsius for hwmon so the offset is
-* -272.15 * scale
+* -273.15 * scale
 */
-   *val = -1075;
-   *val2 = 691699;
+   *val = -1079;
+   *val2 = 644268;

return IIO_VAL_INT_PLUS_MICRO;
}



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] iio: mxs-lradc: Fix temperature offset

2015-10-06 Thread Marek Vasut

On 10/06/2015 20:43, Stefan Wahren wrote:

Hi,


Alexandre Belloni  hat am 6. Oktober
2015 um 16:22 geschrieben:


0° Kelvin is actually −273.15°C, not -272.15°C. Fix the temperature offset.

Reported-by: Janusz Użycki 
Signed-off-by: Alexandre Belloni 
---
drivers/staging/iio/adc/mxs-lradc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c
b/drivers/staging/iio/adc/mxs-lradc.c
index 3f7715c9968b..67bfa54c4fb1 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -916,10 +916,10 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
if (chan->type == IIO_TEMP) {
/* The calculated value from the ADC is in Kelvin, we
* want Celsius for hwmon so the offset is
- * -272.15 * scale
+ * -273.15 * scale
*/
- *val = -1075;
- *val2 = 691699;
+ *val = -1079;
+ *val2 = 644268;



i think it would be nice to break down the calculation of these magic values in
a second patch.


Indeed, I agree. I hope Alexandre can send another patch for that soon :-)


Regards
Stefan



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] crypto: mxs-dcp is an stmp device

2015-10-12 Thread Marek Vasut
On Monday, October 12, 2015 at 03:52:34 PM, Arnd Bergmann wrote:
> The mxs-dcp driver relies on the stmp_reset_block() helper function, which
> is provided by CONFIG_STMP_DEVICE. This symbol is always set on MXS,
> but the driver can now also be built for MXC (i.MX6)

That is correct.

> , which results in a built error if no other driver selects STMP_DEVICE:

Ouch.

> drivers/built-in.o: In function `mxs_dcp_probe':
> vf610-ocotp.c:(.text+0x3df302): undefined reference to `stmp_reset_block'
> 
> This adds the 'select', like all other stmp drivers have it.
> 
> Signed-off-by: Arnd Bergmann 
> Fixes: a2712e6c75f ("crypto: mxs-dcp - Allow MXS_DCP to be used on MX6SL")

Acked-by: Marek Vasut 

> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
> index ab7e3b668890..2569e043317e 100644
> --- a/drivers/crypto/Kconfig
> +++ b/drivers/crypto/Kconfig
> @@ -430,6 +430,7 @@ endif
>  config CRYPTO_DEV_MXS_DCP
>   tristate "Support for Freescale MXS DCP"
>   depends on (ARCH_MXS || ARCH_MXC)
> + select STMP_DEVICE
>   select CRYPTO_CBC
>   select CRYPTO_ECB
>   select CRYPTO_AES

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH linux-next v5 1/5] mtd: spi-nor: notify (Q)SPI controller about protocol change

2015-09-08 Thread Marek Vasut
On Tuesday, September 08, 2015 at 06:20:05 PM, Cyrille Pitchen wrote:
> Hi Jonas,
> 
> taking your comments into account I'm about to test a new series with
> additional patches to handle the Read ID command in multiple I/O protocols
> and relying on new members in the struct spi_nor:
> 
>  * @erase_proto:  the SPI protocol used by erase operations
>  * @read_proto:   the SPI protocol used by read operations
>  * @write_proto:  the SPI protocol used by write operations
>  * @reg_proto the SPI protocol used by read_reg/write_reg operations
> 
>   enum spi_protocol   erase_proto;
>   enum spi_protocol   read_proto;
>   enum spi_protocol   write_proto;
>   enum spi_protocol   reg_proto;
> 
> This way, the read(), write(), erase(), read_reg() and write_reg() hooks
> can check the relevant protocol member so the spi-nor framework doesn't
> need to call spi_nor_set_protocol() before any command.
> 
> Also the op codes for read, page program and erase commands will be tuned
> depending on the memory manufacturer and the selected SPI protocol.
> 
> I'm likely to publish this new series tomorrow after my tests on a Micron
> memory.

Excellent, please keep me on Cc as I'm already warming up my Spansion part ;-)

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH linux-next v6 1/8] mtd: spi-nor: read JEDEC ID with multiple I/O protocols

2015-09-09 Thread Marek Vasut
On Wednesday, September 09, 2015 at 03:24:11 PM, Cyrille Pitchen wrote:
> When their quad or dual I/O mode is enabled, Micron and Macronix spi-nor
> memories don't reply to the regular Read ID (0x9f) command. Instead they
> reply to a new dedicated command Read ID Multiple I/O (0xaf).
> 
> If the Read ID (0x9f) command fails (the read ID is all 1's or all 0's),
> then the Read ID Multiple I/O (0xaf) is used, first with SPI 4-4-4 protocol
> (supported by both Micron and Macronix memories), lately with SPI-2-2-2
> protocol (supported only by Micron memories).
> 
> Signed-off-by: Cyrille Pitchen 
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 28 +++-
>  include/linux/mtd/spi-nor.h   | 23 +--
>  2 files changed, 48 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index f59aedfe1462..80a0db078aaa 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -703,8 +703,9 @@ static const struct flash_info spi_nor_ids[] = {
> 
>  static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
>  {
> - int tmp;
> + int i, tmp;
>   u8  id[SPI_NOR_MAX_ID_LEN];
> + enum spi_protocol   proto[2] = {SPI_PROTO_4_4_4, SPI_PROTO_2_2_2};
>   const struct flash_info *info;
> 
>   tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
> @@ -713,6 +714,25 @@ static const struct flash_info *spi_nor_read_id(struct
> spi_nor *nor) return ERR_PTR(tmp);
>   }
> 
> + /* Special case for Micron/Macronix qspi nor. */
> + for (i = 0; i < ARRAY_SIZE(proto); ++i) {
> + if (!((id[0] == 0xff && id[1] == 0xff && id[2] == 0xff) ||
> +   (id[0] == 0x00 && id[1] == 0x00 && id[2] == 0x00)))
> + break;
> +
> + nor->erase_proto = proto[i];
> + nor->read_proto = proto[i];
> + nor->write_proto = proto[i];
> + nor->reg_proto = proto[i];
> + tmp = nor->read_reg(nor, SPINOR_OP_MIO_RDID,
> + id, SPI_NOR_MAX_ID_LEN);
> + if (tmp < 0) {
> + dev_dbg(nor->dev,
> + " error %d reading JEDEC ID (MULTI IO)\n", tmp);

Don't you have one space too much in front of the " error" ?

> + return ERR_PTR(tmp);
> + }
> + }
> +
>   for (tmp = 0; tmp < ARRAY_SIZE(spi_nor_ids) - 1; tmp++) {
>   info = &spi_nor_ids[tmp];
>   if (info->id_len) {

[...]
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] asm-generic: cmpxchg: avoid warnings from macro-ized cmpxchg() implementations

2015-10-15 Thread Marek Vasut
On Wednesday, October 14, 2015 at 05:01:45 PM, Arnd Bergmann wrote:
> On Saturday 19 September 2015 06:42:21 Marek Vasut wrote:
> > This change is similar to e001bbae7147b111fe1aa42beaf835635f3c016e
> > ARM: cmpxchg: avoid warnings from macro-ized cmpxchg() implementations
> > 
> > A recent change in kernel/acct.c added a new warning for many
> > configurations using generic __xchg() implementation:
> > 
> > In file included from ./arch/nios2/include/asm/cmpxchg.h:12:0,
> > 
> >  from include/asm-generic/atomic.h:18,
> >  from arch/nios2/include/generated/asm/atomic.h:1,
> >  from include/linux/atomic.h:4,
> >  from include/linux/spinlock.h:406,
> >  from include/linux/mmzone.h:7,
> >  from include/linux/gfp.h:5,
> >  from include/linux/mm.h:9,
> > 
> >  from kernel/acct.c:46:
> > kernel/acct.c: In function 'acct_pin_kill':
> > include/asm-generic/cmpxchg.h:94:3: warning: value computed is not used
> > [-Wunused-value]
> > 
> >   ((__typeof__(*(ptr)))__cmpxchg_local_generic((ptr), (unsigned
> >   long)(o),\
> >   
> >^
> > 
> > include/asm-generic/cmpxchg.h:102:28: note: in expansion of macro
> > 'cmpxchg_local'
> > 
> >  #define cmpxchg(ptr, o, n) cmpxchg_local((ptr), (o), (n))
> >  
> > ^
> > 
> > kernel/acct.c:177:2: note: in expansion of macro 'cmpxchg'
> > 
> >   cmpxchg(&acct->ns->bacct, pin, NULL);
> >   ^
> > 
> > The code is in fact correct, it's just a cmpxchg() call that
> > intentionally ignores the result, and no other code does that.  The
> > warning does not show up on x86 because of the way that its cmpxchg()
> > macro is written. This changes the asm-ggeneric implementation to use
> > a similar construct with a compound expression instead of a typecast,
> > which causes the compiler to not complain about an unused result.
> > 
> > Fix the other macros in this file in a similar way, and place them
> > just below their function implementations.
> > 
> > Signed-off-by: Marek Vasut 
> > Cc: Russell King 
> 
> Applied to my asm-generic tree, thanks!

Thank you!

Now I really hope someone doesn't flame me to a crisp for breaking some odd
architecture ;-)

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [LINUX RFC v2 0/4] spi: add dual parallel mode support in Zynq MPSoC GQSPI controller

2015-09-02 Thread Marek Vasut
On Wednesday, September 02, 2015 at 07:12:14 PM, Ranjit Abhimanyu Waghmode 
wrote:
> Hi Marek,
> 
> > -Original Message-
> > From: Marek Vasut [mailto:ma...@denx.de]
> > Sent: Wednesday, August 26, 2015 12:26 PM
> > To: Ranjit Abhimanyu Waghmode
> > Cc: dw...@infradead.org; computersforpe...@gmail.com;
> > broo...@kernel.org; Michal Simek; Soren Brinkmann; zaj...@gmail.com;
> > b...@decadent.org.uk; b32...@freescale.com; knut.wohl...@de.bosch.com;
> > juh...@openwrt.org; bean...@micron.com; linux-...@lists.infradead.org;
> > linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; linux-arm-
> > ker...@lists.infradead.org; Harini Katakam; Punnaiah Choudary Kalluri
> > Subject: Re: [LINUX RFC v2 0/4] spi: add dual parallel mode support in
> > Zynq MPSoC GQSPI controller
> > 
> > On Wednesday, August 26, 2015 at 08:26:03 AM, Ranjit Waghmode wrote:
> > > This series adds dual parallel mode support for Zynq Ultrascale+ MPSoC
> > > GQSPI controller driver.
> > > 
> > > What is dual parallel mode?
> > > ---
> > > ZynqMP GQSPI controller supports Dual Parallel mode with following
> > > functionalities: 1) Supporting two SPI flash memories operating in
> > > parallel. 8 I/O lines. 2) Chip selects and clock are shared to both
> > > the flash devices
> > > 3) This mode is targeted for faster read/write speed and also doubles
> > > the size 4) Commands/data can be transmitted/received from both the
> > > devices(mirror), or only upper or only lower flash memory devices.
> > > 
> > > 5) Data arrangement:
> > >With stripe enabled,
> > >Even bytes i.e. 0, 2, 4,... are transmitted on Lower Data Bus
> > >Odd bytes i.e. 1, 3, 5,.. are transmitted on Upper Data Bus.
> > 
> > This might be a dumb question, but why don't you just treat this as an
> > SPI NOR flash with 8-bit bus ?
> 
> In case of dual parallel configuration of this controller there are
> different modes like single, dual and quad mode. Whatever you are
> suggesting would fit only in the case of Quad mode operation as both buses
> would have 4 lines each. In case of single mode of parallel configuration,
> there would be two buses; but the line on each bus would one. So
> altogether there will be two lines. And in case of dual mode of parallel
> configuration each bus will be having two lines. So altogether 4 lines
> will be there. So keeping 8 lines would not support above two modes of
> parallel configuration correctly.
> 
> Logically it is a single flash with 8 IO lines but physically it's a two
> flash devices and each has 4 IO lines. So, in this case, read and write
> addresses should be always even and minimum data that can be accessed is 2
> bytes.

Oh, I see what the issue is now. It has to do with configuring the flash
into correct bus-width mode, right ?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [LINUX RFC v2 0/4] spi: add dual parallel mode support in Zynq MPSoC GQSPI controller

2015-09-03 Thread Marek Vasut
On Thursday, September 03, 2015 at 03:25:00 PM, Ranjit Abhimanyu Waghmode wrote:
> Hi,
> 
> > -Original Message-
> > From: Marek Vasut [mailto:ma...@denx.de]
> > Sent: Thursday, September 03, 2015 12:26 AM
> > To: Ranjit Abhimanyu Waghmode
> > Cc: dw...@infradead.org; computersforpe...@gmail.com;
> > broo...@kernel.org; Michal Simek; Soren Brinkmann; zaj...@gmail.com;
> > b...@decadent.org.uk; b32...@freescale.com; knut.wohl...@de.bosch.com;
> > juh...@openwrt.org; bean...@micron.com; linux-...@lists.infradead.org;
> > linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; linux-arm-
> > ker...@lists.infradead.org; Harini Katakam; Punnaiah Choudary Kalluri
> > Subject: Re: [LINUX RFC v2 0/4] spi: add dual parallel mode support in
> > Zynq MPSoC GQSPI controller
> > 
> > On Wednesday, September 02, 2015 at 07:12:14 PM, Ranjit Abhimanyu
> > Waghmode
> > 
> > wrote:
> > > Hi Marek,
> > > 
> > > > -Original Message-
> > > > From: Marek Vasut [mailto:ma...@denx.de]
> > > > Sent: Wednesday, August 26, 2015 12:26 PM
> > > > To: Ranjit Abhimanyu Waghmode
> > > > Cc: dw...@infradead.org; computersforpe...@gmail.com;
> > > > broo...@kernel.org; Michal Simek; Soren Brinkmann; zaj...@gmail.com;
> > > > b...@decadent.org.uk; b32...@freescale.com;
> > > > knut.wohl...@de.bosch.com; juh...@openwrt.org;
> > 
> > bean...@micron.com;
> > 
> > > > linux-...@lists.infradead.org; linux-kernel@vger.kernel.org;
> > > > linux-...@vger.kernel.org; linux-arm- ker...@lists.infradead.org;
> > > > Harini Katakam; Punnaiah Choudary Kalluri
> > > > Subject: Re: [LINUX RFC v2 0/4] spi: add dual parallel mode support
> > > > in Zynq MPSoC GQSPI controller
> > > > 
> > > > On Wednesday, August 26, 2015 at 08:26:03 AM, Ranjit Waghmode wrote:
> > > > > This series adds dual parallel mode support for Zynq Ultrascale+
> > > > > MPSoC GQSPI controller driver.
> > > > > 
> > > > > What is dual parallel mode?
> > > > > ---
> > > > > ZynqMP GQSPI controller supports Dual Parallel mode with following
> > > > > functionalities: 1) Supporting two SPI flash memories operating in
> > > > > parallel. 8 I/O lines. 2) Chip selects and clock are shared to
> > > > > both the flash devices
> > > > > 3) This mode is targeted for faster read/write speed and also
> > > > > doubles the size 4) Commands/data can be transmitted/received from
> > > > > both the devices(mirror), or only upper or only lower flash memory
> > 
> > devices.
> > 
> > > > > 5) Data arrangement:
> > > > >With stripe enabled,
> > > > >Even bytes i.e. 0, 2, 4,... are transmitted on Lower Data Bus
> > > > >Odd bytes i.e. 1, 3, 5,.. are transmitted on Upper Data Bus.
> > > > 
> > > > This might be a dumb question, but why don't you just treat this as
> > > > an SPI NOR flash with 8-bit bus ?
> > > 
> > > In case of dual parallel configuration of this controller there are
> > > different modes like single, dual and quad mode. Whatever you are
> > > suggesting would fit only in the case of Quad mode operation as both
> > > buses would have 4 lines each. In case of single mode of parallel
> > > configuration, there would be two buses; but the line on each bus
> > > would one. So altogether there will be two lines. And in case of dual
> > > mode of parallel configuration each bus will be having two lines. So
> > > altogether 4 lines will be there. So keeping 8 lines would not support
> > > above two modes of parallel configuration correctly.
> > > 
> > > Logically it is a single flash with 8 IO lines but physically it's a
> > > two flash devices and each has 4 IO lines. So, in this case, read and
> > > write addresses should be always even and minimum data that can be
> > > accessed is 2 bytes.
> > 
> > Oh, I see what the issue is now. It has to do with configuring the flash
> > into correct bus-width mode, right ?
> 
> Yes.

Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] spi-nor: add support for is25wp256d

2018-08-04 Thread Marek Vasut
On 08/04/2018 03:49 AM, Palmer Dabbelt wrote:
> From: "Wesley W. Terpstra" 
> 
> This is used of the HiFive Unleashed development board.
> 
> Signed-off-by: Wesley W. Terpstra 
> Signed-off-by: Palmer Dabbelt 
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 47 
> ++-
>  include/linux/mtd/spi-nor.h   |  2 ++
>  2 files changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index d9c368c44194..e9a3557a3c23 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1072,6 +1072,9 @@ static const struct flash_info spi_nor_ids[] = {
>   SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>   { "is25wp128",  INFO(0x9d7018, 0, 64 * 1024, 256,
>   SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + { "is25wp256d", INFO(0x9d7019, 0, 32 * 1024, 1024,

Is there a reason for the trailing 'd' in is25wp256d ? I'd drop it.

> + SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | 
> SPI_NOR_4B_OPCODES)
> + },
>  
>   /* Macronix */
>   { "mx25l512e",   INFO(0xc22010, 0, 64 * 1024,   1, SECT_4K) },
> @@ -1515,6 +1518,45 @@ static int macronix_quad_enable(struct spi_nor *nor)
>   return 0;
>  }
>  
> +/**
> + * issi_unlock() - clear BP[0123] write-protection.
> + * @nor: pointer to a 'struct spi_nor'
> + *
> + * Bits [2345] of the Status Register are BP[0123].
> + * ISSI chips use a different block protection scheme than other chips.
> + * Just disable the write-protect unilaterally.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int issi_unlock(struct spi_nor *nor)
> +{
> + int ret, val;
> + u8 mask = SR_BP0 | SR_BP1 | SR_BP2 | SR_BP3;
> +
> + val = read_sr(nor);
> + if (val < 0)
> + return val;
> + if (!(val & mask))
> + return 0;
> +
> + write_enable(nor);
> +
> + write_sr(nor, val & ~mask);
> +
> + ret = spi_nor_wait_till_ready(nor);
> + if (ret)
> + return ret;
> +
> + ret = read_sr(nor);
> + if (ret > 0 && !(ret & mask)) {
> + dev_info(nor->dev, "ISSI Block Protection Bits cleared\n");
> + return 0;

Is the dev_info() really needed ?

> + } else {
> + dev_err(nor->dev, "ISSI Block Protection Bits not cleared\n");
> + return -EINVAL;
> + }
> +}

[...]
-- 
Best regards,
Marek Vasut


Re: [PATCH] spi-nor: add support for is25wp256d

2018-08-06 Thread Marek Vasut
On 08/06/2018 10:58 PM, Palmer Dabbelt wrote:
> On Sat, 04 Aug 2018 02:27:54 PDT (-0700), marek.va...@gmail.com wrote:
>> On 08/04/2018 03:49 AM, Palmer Dabbelt wrote:
>>> From: "Wesley W. Terpstra" 
>>>
>>> This is used of the HiFive Unleashed development board.
>>>
>>> Signed-off-by: Wesley W. Terpstra 
>>> Signed-off-by: Palmer Dabbelt 
>>> ---
>>>  drivers/mtd/spi-nor/spi-nor.c | 47
>>> ++-
>>>  include/linux/mtd/spi-nor.h   |  2 ++
>>>  2 files changed, 48 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c
>>> b/drivers/mtd/spi-nor/spi-nor.c
>>> index d9c368c44194..e9a3557a3c23 100644
>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>> @@ -1072,6 +1072,9 @@ static const struct flash_info spi_nor_ids[] = {
>>>  SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>>>  { "is25wp128",  INFO(0x9d7018, 0, 64 * 1024, 256,
>>>  SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>>> +    { "is25wp256d", INFO(0x9d7019, 0, 32 * 1024, 1024,
>>
>> Is there a reason for the trailing 'd' in is25wp256d ? I'd drop it.
> 
> I'm honestly not sure.  There are data sheets for both of them, but I
> don't see much of a difference
> 
>    http://www.issi.com/WW/pdf/IS25LP(WP)256D.pdf
>    http://www.issi.com/WW/pdf/25LP-WP256.pdf
> 
> Following the pattern, I'd expect to see
> 
>    { "is25wp256",  INFO(0x9d7019, 0, 64 * 1024, 512,
>    SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> 
> versus
> 
>    { "is25wp256d", INFO(0x9d7019, 0, 32 * 1024, 1024,
>    SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> SPI_NOR_4B_OPCODES)
>    },

They have the same ID ? Do we support the variant without the d already?

> So in other words: the d less sections that are larger, and also has the
> 4B opcodes flag set.  From the documentation in looks like the non-d
> version supports 3 and 4 byte opcodes, so I guess it's just a different
> physical layout?
> 
> In the data sheet for both I see
> 
>    "Pages can be erased in groups of 4Kbyte sectors, 32Kbyte blocks,
> 64Kbyte    blocks, and/or the entire chip"
> 
> which indicates to me that maybe we've just selected the larger section
> size?  If so then I'll change it to the first one in the new patch.
> 
>>> +    SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ
>>> | SPI_NOR_4B_OPCODES)
>>> +    },
>>>
>>>  /* Macronix */
>>>  { "mx25l512e",   INFO(0xc22010, 0, 64 * 1024,   1, SECT_4K) },
>>> @@ -1515,6 +1518,45 @@ static int macronix_quad_enable(struct spi_nor
>>> *nor)
>>>  return 0;
>>>  }
>>>
>>> +/**
>>> + * issi_unlock() - clear BP[0123] write-protection.
>>> + * @nor:    pointer to a 'struct spi_nor'
>>> + *
>>> + * Bits [2345] of the Status Register are BP[0123].
>>> + * ISSI chips use a different block protection scheme than other chips.
>>> + * Just disable the write-protect unilaterally.
>>> + *
>>> + * Return: 0 on success, -errno otherwise.
>>> + */
>>> +static int issi_unlock(struct spi_nor *nor)
>>> +{
>>> +    int ret, val;
>>> +    u8 mask = SR_BP0 | SR_BP1 | SR_BP2 | SR_BP3;
>>> +
>>> +    val = read_sr(nor);
>>> +    if (val < 0)
>>> +    return val;
>>> +    if (!(val & mask))
>>> +    return 0;
>>> +
>>> +    write_enable(nor);
>>> +
>>> +    write_sr(nor, val & ~mask);
>>> +
>>> +    ret = spi_nor_wait_till_ready(nor);
>>> +    if (ret)
>>> +    return ret;
>>> +
>>> +    ret = read_sr(nor);
>>> +    if (ret > 0 && !(ret & mask)) {
>>> +    dev_info(nor->dev, "ISSI Block Protection Bits cleared\n");
>>> +    return 0;
>>
>> Is the dev_info() really needed ?
> 
> Nope.  I'll spin a v2 pending the above discussion.
> 
>>> +    } else {
>>> +    dev_err(nor->dev, "ISSI Block Protection Bits not cleared\n");
>>> +    return -EINVAL;
>>> +    }
>>> +}
>>
>> [...]
> 
> Thanks!


-- 
Best regards,
Marek Vasut


Re: [PATCH] spi-nor: add support for is25wp256d

2018-08-07 Thread Marek Vasut
>>>>
>>>>>  /* Macronix */
>>>>>  { "mx25l512e",   INFO(0xc22010, 0, 64 * 1024,   1, SECT_4K) },
>>>>> @@ -1515,6 +1518,45 @@ static int macronix_quad_enable(struct spi_nor
>>>>> *nor)
>>>>>  return 0;
>>>>>  }
>>>>>
>>>>> +/**
>>>>> + * issi_unlock() - clear BP[0123] write-protection.
>>>>> + * @nor:    pointer to a 'struct spi_nor'
>>>>> + *
>>>>> + * Bits [2345] of the Status Register are BP[0123].
>>>>> + * ISSI chips use a different block protection scheme than other
>>>>> chips.
>>>>> + * Just disable the write-protect unilaterally.
>>>>> + *
>>>>> + * Return: 0 on success, -errno otherwise.
>>>>> + */
>>>>> +static int issi_unlock(struct spi_nor *nor)
>>>>> +{
>>>>> +    int ret, val;
>>>>> +    u8 mask = SR_BP0 | SR_BP1 | SR_BP2 | SR_BP3;
>>>>> +
>>>>> +    val = read_sr(nor);
>>>>> +    if (val < 0)
>>>>> +    return val;
>>>>> +    if (!(val & mask))
>>>>> +    return 0;
>>>>> +
>>>>> +    write_enable(nor);
>>>>> +
>>>>> +    write_sr(nor, val & ~mask);
>>>>> +
>>>>> +    ret = spi_nor_wait_till_ready(nor);
>>>>> +    if (ret)
>>>>> +    return ret;
>>>>> +
>>>>> +    ret = read_sr(nor);
>>>>> +    if (ret > 0 && !(ret & mask)) {
>>>>> +    dev_info(nor->dev, "ISSI Block Protection Bits cleared\n");
>>>>> +    return 0;
>>>>
>>>> Is the dev_info() really needed ?
>>>
>>> Nope.  I'll spin a v2 pending the above discussion.
>>>
>>>>> +    } else {
>>>>> +    dev_err(nor->dev, "ISSI Block Protection Bits not
>>>>> cleared\n");
>>>>> +    return -EINVAL;
>>>>> +    }
>>>>> +}
>>>>
>>>> [...]
>>>
>>> Thanks!


-- 
Best regards,
Marek Vasut


Re: [PATCH] arm: dts: socfpga: denali needs nand_x_clk too

2018-06-20 Thread Marek Vasut
On 06/20/2018 07:27 AM, Richard Weinberger wrote:
> Marek,
> 
> Am Mittwoch, 20. Juni 2018, 06:52:09 CEST schrieb Marek Vasut:
>> On 06/19/2018 02:07 PM, Richard Weinberger wrote:
>>> The denali NAND flash controller needs at least two clocks to operate,
>>> nand_clk and nand_x_clk.
>>> Since 1bb88666775e ("mtd: nand: denali: handle timing parameters by
>>> setup_data_interface()") nand_x_clk is used to derive timing settings.
>>>
>>> Signed-off-by: Richard Weinberger 
>>> ---
>>> Strictly speaking denali needs a ecc_clk too, but AFAIK such a clock
>>> is not present on this SoC.
>>> But my SoCFPGA knowledge is very limited.
>>>
>>> Thanks,
>>> //richard
>>
>> It looks sane, but I cannot test it right now, since I'm on vacation.
>> I hope Dinh/Chin can jump in.
> 
> The patch was tested by me. So, at least it is not completely untested.
> BTW: I forgot to mention that it depends on Masahiro's Denali fixes.

Then that's perfect.

Reviewed-by: Marek Vasut 

-- 
Best regards,
Marek Vasut


Re: [PATCH] ARM: dts: socfpga: use stdout-path for chosen node

2018-08-08 Thread Marek Vasut
On 08/08/2018 11:09 AM, Simon Goldschmidt wrote:
> Use stdout-path dts property for kernel console.
> 
> There were two socfpga boards left not using stdout-path:
> socrates and vining. Make sure they match the other boards.
> 
> Signed-off-by: Simon Goldschmidt 
> ---
>  arch/arm/boot/dts/socfpga_cyclone5_socrates.dts| 3 ++-
>  arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts 
> b/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts
> index 53bf99eef66d..6f5255a7d192 100644
> --- a/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts
> +++ b/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts
> @@ -22,7 +22,8 @@
>   compatible = "ebv,socrates", "altr,socfpga-cyclone5", "altr,socfpga";
>  
>   chosen {
> - bootargs = "console=ttyS0,115200";
> + bootargs = "earlyprintk";
> + stdout-path = "serial0:115200n8";
>   };
>  
>   memory@0 {
> diff --git a/arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts 
> b/arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts
> index f50b19447de6..e61efe16e79c 100644
> --- a/arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts
> +++ b/arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts
> @@ -54,7 +54,8 @@
>   compatible = "samtec,vining", "altr,socfpga-cyclone5", "altr,socfpga";
>  
>   chosen {
> - bootargs = "console=ttyS0,115200";
> + bootargs = "earlyprintk";

Why this earlyprintk ?

> + stdout-path = "serial0:115200n8";
>   };
>  
>   memory@0 {
> 


-- 
Best regards,
Marek Vasut


Re: [PATCH] ARM: dts: socfpga: use stdout-path for chosen node

2018-08-08 Thread Marek Vasut
On 08/08/2018 02:02 PM, Simon Goldschmidt wrote:
> On Wed, Aug 8, 2018 at 1:32 PM Marek Vasut  wrote:
>>
>> On 08/08/2018 11:09 AM, Simon Goldschmidt wrote:
>>> Use stdout-path dts property for kernel console.
>>>
>>> There were two socfpga boards left not using stdout-path:
>>> socrates and vining. Make sure they match the other boards.
>>>
>>> Signed-off-by: Simon Goldschmidt 
>>> ---
>>>  arch/arm/boot/dts/socfpga_cyclone5_socrates.dts| 3 ++-
>>>  arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts | 3 ++-
>>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts 
>>> b/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts
>>> index 53bf99eef66d..6f5255a7d192 100644
>>> --- a/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts
>>> +++ b/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts
>>> @@ -22,7 +22,8 @@
>>>   compatible = "ebv,socrates", "altr,socfpga-cyclone5", "altr,socfpga";
>>>
>>>   chosen {
>>> - bootargs = "console=ttyS0,115200";
>>> + bootargs = "earlyprintk";
>>> + stdout-path = "serial0:115200n8";
>>>   };
>>>
>>>   memory@0 {
>>> diff --git a/arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts 
>>> b/arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts
>>> index f50b19447de6..e61efe16e79c 100644
>>> --- a/arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts
>>> +++ b/arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts
>>> @@ -54,7 +54,8 @@
>>>   compatible = "samtec,vining", "altr,socfpga-cyclone5", "altr,socfpga";
>>>
>>>   chosen {
>>> - bootargs = "console=ttyS0,115200";
>>> + bootargs = "earlyprintk";
>>
>> Why this earlyprintk ?
> 
> The reason is Dinh added that to all socfpga boards in his commit [1]
> and I wanted the remaining two boards to be the same.

Isn't earlyprintk mostly a debugging aid ?

-- 
Best regards,
Marek Vasut


Re: [PATCH] ARM: spectre-v2: Try to set IBE bit for Cortex-A15 and Brahma-B15

2018-08-11 Thread Marek Vasut
On 06/08/2018 12:58 AM, Florian Fainelli wrote:
> Per the ARM reference manual for the Cortex-A15, The ACTLR:
> 
>  Is a read/write register.
> 
>  Common to the Secure and Non-secure states.
> 
>  Is only accessible from PL1 or higher, with access rights that depend
>  on the mode:
> 
> * Read/write in Secure PL1 modes.
> 
> * Read-only and write-ignored in Non-secure PL1 and PL2 modes
>   if NSACR.NS_SMP is 0.
> 
> * Read/write in Non-secure PL1 and PL2 modes if NSACR.NS_SMP
>   is 1. In this case, all bits are write-ignored except for the SMP bit.
> 
> We can attempt to set this bit from within the kernel, which helps
> avoiding firmware side modifications to set the IBE bit when that is
> impractical. We do this within __v7_ca15mp_setup and __v7_b15mp_setup
> because by then we already took those labels because the processors we
> run on do match.
> 
> Signed-off-by: Florian Fainelli 

I believe this would help on selected R-Car Gen2 SoCs which have CA15
cores. Those cores are released from reset by Linux directly and jump to
the reset vector set up by Linux in Secure mode.

I was looking for a generic solution after discussing how to mitigate
the spectrev2 on such SoCs with Catalin on IRC and this seems to be one
option. There was also a suggestion that Russell is working on some sort
of generic solution too. Can you comment on that please ?

> ---
>  arch/arm/mm/proc-v7.S | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> index 6fe52819e014..a21cf3729efa 100644
> --- a/arch/arm/mm/proc-v7.S
> +++ b/arch/arm/mm/proc-v7.S
> @@ -284,10 +284,16 @@ __v7_cr8mp_setup:
>   b   1f
>  __v7_ca7mp_setup:
>  __v7_ca12mp_setup:
> + b   2f
>  __v7_ca15mp_setup:
>  __v7_b15mp_setup:
> +#ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
> + mrc p15, 0, r0, c1, c0, 1
> + orr r0, r0, #1  @ Enable IBE bit
> + mcr p15, 0, r0, c1, c0, 1
> +#endif
>  __v7_ca17mp_setup:
> - mov r10, #0
> +2:   mov r10, #0
>  1:   adr r0, __v7_setup_stack_ptr
>   ldr r12, [r0]
>   add r12, r12, r0@ the local stack
> 


-- 
Best regards,
Marek Vasut


Re: [PATCH] ARM: spectre-v2: Try to set IBE bit for Cortex-A15 and Brahma-B15

2018-08-22 Thread Marek Vasut
On 08/11/2018 03:58 PM, Marek Vasut wrote:
> On 06/08/2018 12:58 AM, Florian Fainelli wrote:
>> Per the ARM reference manual for the Cortex-A15, The ACTLR:
>>
>>  Is a read/write register.
>>
>>  Common to the Secure and Non-secure states.
>>
>>  Is only accessible from PL1 or higher, with access rights that depend
>>  on the mode:
>>
>> * Read/write in Secure PL1 modes.
>>
>> * Read-only and write-ignored in Non-secure PL1 and PL2 modes
>>   if NSACR.NS_SMP is 0.
>>
>> * Read/write in Non-secure PL1 and PL2 modes if NSACR.NS_SMP
>>   is 1. In this case, all bits are write-ignored except for the SMP bit.
>>
>> We can attempt to set this bit from within the kernel, which helps
>> avoiding firmware side modifications to set the IBE bit when that is
>> impractical. We do this within __v7_ca15mp_setup and __v7_b15mp_setup
>> because by then we already took those labels because the processors we
>> run on do match.
>>
>> Signed-off-by: Florian Fainelli 
> 
> I believe this would help on selected R-Car Gen2 SoCs which have CA15
> cores. Those cores are released from reset by Linux directly and jump to
> the reset vector set up by Linux in Secure mode.
> 
> I was looking for a generic solution after discussing how to mitigate
> the spectrev2 on such SoCs with Catalin on IRC and this seems to be one
> option. There was also a suggestion that Russell is working on some sort
> of generic solution too. Can you comment on that please ?

Is there any feedback on this ?

>> ---
>>  arch/arm/mm/proc-v7.S | 8 +++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
>> index 6fe52819e014..a21cf3729efa 100644
>> --- a/arch/arm/mm/proc-v7.S
>> +++ b/arch/arm/mm/proc-v7.S
>> @@ -284,10 +284,16 @@ __v7_cr8mp_setup:
>>  b   1f
>>  __v7_ca7mp_setup:
>>  __v7_ca12mp_setup:
>> +b   2f
>>  __v7_ca15mp_setup:
>>  __v7_b15mp_setup:
>> +#ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
>> +mrc p15, 0, r0, c1, c0, 1
>> +orr r0, r0, #1  @ Enable IBE bit
>> +    mcr p15, 0, r0, c1, c0, 1
>> +#endif
>>  __v7_ca17mp_setup:
>> -mov r10, #0
>> +2:  mov r10, #0
>>  1:  adr r0, __v7_setup_stack_ptr
>>  ldr r12, [r0]
>>  add r12, r12, r0@ the local stack
>>
> 
> 


-- 
Best regards,
Marek Vasut


Re: [PATCH 5/5] crypto: ahash.c: Require export/import in ahash

2018-01-18 Thread Marek Vasut
On 01/18/2018 07:34 PM, Kamil Konieczny wrote:
> Export and import are mandatory in async hash. As drivers were
> rewritten, drop empty wrappers and correct init of ahash transformation.

Are you moving checks from the core subsystem to drivers ? This looks
really nonsensical and the commit message doesn't explain the rationale
for that at all.

> Signed-off-by: Kamil Konieczny 
> ---
>  crypto/ahash.c | 18 ++
>  1 file changed, 2 insertions(+), 16 deletions(-)
> 
> diff --git a/crypto/ahash.c b/crypto/ahash.c
> index 3a35d67de7d9..c3cce508c1d4 100644
> --- a/crypto/ahash.c
> +++ b/crypto/ahash.c
> @@ -434,16 +434,6 @@ static int ahash_def_finup(struct ahash_request *req)
>   return ahash_def_finup_finish1(req, err);
>  }
>  
> -static int ahash_no_export(struct ahash_request *req, void *out)
> -{
> - return -ENOSYS;
> -}
> -
> -static int ahash_no_import(struct ahash_request *req, const void *in)
> -{
> - return -ENOSYS;
> -}
> -
>  static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
>  {
>   struct crypto_ahash *hash = __crypto_ahash_cast(tfm);
> @@ -451,8 +441,6 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
>  
>   hash->setkey = ahash_nosetkey;
>   hash->has_setkey = false;
> - hash->export = ahash_no_export;
> - hash->import = ahash_no_import;
>  
>   if (tfm->__crt_alg->cra_type != &crypto_ahash_type)
>   return crypto_init_shash_ops_async(tfm);
> @@ -462,15 +450,13 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
>   hash->final = alg->final;
>   hash->finup = alg->finup ?: ahash_def_finup;
>   hash->digest = alg->digest;
> + hash->export = alg->export;
> + hash->import = alg->import;
>  
>   if (alg->setkey) {
>   hash->setkey = alg->setkey;
>   hash->has_setkey = true;
>   }
> - if (alg->export)
> - hash->export = alg->export;
> - if (alg->import)
> - hash->import = alg->import;
>  
>   return 0;
>  }
> 


-- 
Best regards,
Marek Vasut


Re: [PATCH 5/5] crypto: ahash.c: Require export/import in ahash

2018-01-19 Thread Marek Vasut
On 01/19/2018 10:53 AM, Kamil Konieczny wrote:
> On 18.01.2018 22:31, Marek Vasut wrote:
>> On 01/18/2018 07:34 PM, Kamil Konieczny wrote:
>>> Export and import are mandatory in async hash. As drivers were
>>> rewritten, drop empty wrappers and correct init of ahash transformation.
>>
>> Are you moving checks from the core subsystem to drivers ? This looks
>> really nonsensical and the commit message doesn't explain the rationale
>> for that at all.
> 
> I am removing checks from core. Export and import were optional in beginnig
> of crypto framework, but as time goes on they become mandatory.

Seems like if the driver doesn't implement those, the core can easily
detect that and perform the necessary action. Moving the checks out of
core seems like the wrong thing to do, rather you should enhance the
checks in core if they're insufficient in my opinion.

>>
>>> Signed-off-by: Kamil Konieczny 
>>> ---
>>>  crypto/ahash.c | 18 ++
>>>  1 file changed, 2 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/crypto/ahash.c b/crypto/ahash.c
>>> index 3a35d67de7d9..c3cce508c1d4 100644
>>> --- a/crypto/ahash.c
>>> +++ b/crypto/ahash.c
>>> @@ -434,16 +434,6 @@ static int ahash_def_finup(struct ahash_request *req)
>>> return ahash_def_finup_finish1(req, err);
>>>  }
>>>  
>>> -static int ahash_no_export(struct ahash_request *req, void *out)
>>> -{
>>> -   return -ENOSYS;
>>> -}
>>> -
>>> -static int ahash_no_import(struct ahash_request *req, const void *in)
>>> -{
>>> -   return -ENOSYS;
>>> -}
>>> -
>>>  static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
>>>  {
>>> struct crypto_ahash *hash = __crypto_ahash_cast(tfm);
>>> @@ -451,8 +441,6 @@ static int crypto_ahash_init_tfm(struct crypto_tfm *tfm)
>>>  
>>> hash->setkey = ahash_nosetkey;
>>> hash->has_setkey = false;
>>> -   hash->export = ahash_no_export;
>>> -   hash->import = ahash_no_import;
>>>  
>>> if (tfm->__crt_alg->cra_type != &crypto_ahash_type)
>>> return crypto_init_shash_ops_async(tfm);
>>> @@ -462,15 +450,13 @@ static int crypto_ahash_init_tfm(struct crypto_tfm 
>>> *tfm)
>>> hash->final = alg->final;
>>> hash->finup = alg->finup ?: ahash_def_finup;
>>> hash->digest = alg->digest;
>>> +   hash->export = alg->export;
>>> +   hash->import = alg->import;
>>>  
>>> if (alg->setkey) {
>>> hash->setkey = alg->setkey;
>>> hash->has_setkey = true;
>>> }
>>> -   if (alg->export)
>>> -   hash->export = alg->export;
>>> -   if (alg->import)
>>> -   hash->import = alg->import;
>>>  
>>> return 0;
>>>  }
>>>
>>
>>
> 


-- 
Best regards,
Marek Vasut


Re: [PATCH] crypto: mxs-dcp: Add empty hash export and import

2018-01-16 Thread Marek Vasut
On 01/16/2018 05:16 PM, Kamil Konieczny wrote:
> Crypto framework will require async hash export/import, so add empty
> functions to prevent OOPS.

Shouldn't this be handled on the subsystem level with some

if (foo->bar)
 foo->bar();

instead?

> Signed-off-by: Kamil Konieczny 
> ---
>  drivers/crypto/mxs-dcp.c | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
> index 764be3e6933c..a10c418d4e5c 100644
> --- a/drivers/crypto/mxs-dcp.c
> +++ b/drivers/crypto/mxs-dcp.c
> @@ -759,6 +759,16 @@ static int dcp_sha_digest(struct ahash_request *req)
>   return dcp_sha_finup(req);
>  }
>  
> +static int dcp_sha_noimport(struct ahash_request *req, const void *in)
> +{
> + return -ENOSYS;
> +}
> +
> +static int dcp_sha_noexport(struct ahash_request *req, void *out)
> +{
> + return -ENOSYS;
> +}
> +
>  static int dcp_sha_cra_init(struct crypto_tfm *tfm)
>  {
>   crypto_ahash_set_reqsize(__crypto_ahash_cast(tfm),
> @@ -829,6 +839,8 @@ static struct ahash_alg dcp_sha1_alg = {
>   .final  = dcp_sha_final,
>   .finup  = dcp_sha_finup,
>   .digest = dcp_sha_digest,
> + .import = dcp_sha_noimport,
> + .export = dcp_sha_noexport,
>   .halg   = {
>   .digestsize = SHA1_DIGEST_SIZE,
>   .base   = {
> @@ -853,6 +865,8 @@ static struct ahash_alg dcp_sha256_alg = {
>   .final  = dcp_sha_final,
>   .finup  = dcp_sha_finup,
>   .digest = dcp_sha_digest,
> + .import = dcp_sha_noimport,
> + .export = dcp_sha_noexport,
>   .halg   = {
>   .digestsize = SHA256_DIGEST_SIZE,
>   .base   = {
> 


-- 
Best regards,
Marek Vasut


Re: [PATCH] mtd: spi-nor: clear Extended Address Reg on switch to 3-byte addressing.

2018-04-08 Thread Marek Vasut
On 04/08/2018 09:04 AM, NeilBrown wrote:
> 
> According to section
>8.2.7 Write Extended Address Register (C5h)
> 
> of the Winbond W25Q256FV data sheet (256M-BIT SPI flash)
> 
>The Extended Address Register is only effective when the device is
>in the 3-Byte Address Mode.  When the device operates in the 4-Byte
>Address Mode (ADS=1), any command with address input of A31-A24
>will replace the Extended Address Register values. It is
>recommended to check and update the Extended Address Register if
>necessary when the device is switched from 4-Byte to 3-Byte Address
>Mode.
> 
> This patch adds code to implement that recommendation.  Without this,
> my GNUBEE-PC1 will not successfully reboot, as the Extended Address
> Register is left with a value of '1'. When the SOC attempts to read
> (in 3-byte address mode) the boot loader, it reads from the wrong
> location.

Your board is broken by design and does not implement proper reset logic
for the SPI NOR chip, right ? That is, when the CPU resets, the SPI NOR
is left in some random undefined state instead of being reset too, yes?

Doesn't this chip support 4-byte addressing opcodes ? If so, we should
use those and keep the chip in 3-byte addressing mode. Would that work?

> Signed-off-by: NeilBrown 
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 10 ++
>  include/linux/mtd/spi-nor.h   |  2 ++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index d445a4d3b770..c303bf0d2982 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -269,6 +269,7 @@ static inline int set_4byte(struct spi_nor *nor, const 
> struct flash_info *info,
>   int status;
>   bool need_wren = false;
>   u8 cmd;
> + u8 val;
>  
>   switch (JEDEC_MFR(info)) {
>   case SNOR_MFR_MICRON:
> @@ -283,6 +284,15 @@ static inline int set_4byte(struct spi_nor *nor, const 
> struct flash_info *info,
>   status = nor->write_reg(nor, cmd, NULL, 0);
>   if (need_wren)
>   write_disable(nor);
> + if (!status && !enable &&
> + nor->read_reg(nor, SPINOR_OP_RDXA, &val, 1) == 0 &&
> + val != 0) {
> + /* need to reset the Extended Address Register */
> + write_enable(nor);
> + val = 0;
> + nor->write_reg(nor, SPINOR_OP_WRXA, &val, 1);
> + write_disable(nor);
> + }
>  
>   return status;
>   default:
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index de36969eb359..42954419cfdf 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -62,6 +62,8 @@
>  #define SPINOR_OP_RDCR   0x35/* Read configuration register 
> */
>  #define SPINOR_OP_RDFSR  0x70/* Read flag status register */
>  #define SPINOR_OP_CLFSR  0x50/* Clear flag status register */
> +#define SPINOR_OP_RDXA   0xc8/* Read Extended Address 
> Register */
> +#define SPINOR_OP_WRXA   0xc5/* Write Extended Address 
> Register */
>  
>  /* 4-byte address opcodes - used on Spansion and some Macronix flashes. */
>  #define SPINOR_OP_READ_4B0x13/* Read data bytes (low frequency) */
> 


-- 
Best regards,
Marek Vasut


Re: [PATCHv2] mtd: spi-nor: Fix Cadence QSPI page fault kernel panic

2018-04-11 Thread Marek Vasut
On 04/11/2018 05:17 PM, Thor Thayer wrote:
> Hi. Any comments on this patch?

None other than

Reviewed-by: Marek Vasut 

sorry for the delay.

btw stop top-posting please.

> On 03/26/2018 09:12 AM, thor.tha...@linux.intel.com wrote:
>> From: Thor Thayer 
>>
>> The current Cadence QSPI driver caused a kernel panic when loading
>> a Root Filesystem from QSPI. The problem was caused by reading more
>> bytes than needed because the QSPI operated on 4 bytes at a time.
>> 
>> [    7.947754] spi_nor_read[1048]:from 0x037cad74, len 1 [bfe07fff]
>> [    7.956247] cqspi_read[910]:offset 0x58502516, buffer=bfe07fff
>> [    7.956247]
>> [    7.966046] Unable to handle kernel paging request at virtual
>> address bfe08002
>> [    7.973239] pgd = eebfc000
>> [    7.975931] [bfe08002] *pgd=2fffb811, *pte=, *ppte=
>> 
>> Notice above how only 1 byte needed to be read but by reading 4 bytes
>> into the end of a mapped page, an unrecoverable page fault occurred.
>>
>> This patch uses a temporary buffer to hold the 4 bytes read and then
>> copies only the bytes required into the buffer. A min() function is
>> used to limit the length to prevent buffer overflows.
>>
>> Request testing of this patch on other platforms. This was tested
>> on the Intel Arria10 SoCFPGA DevKit.
>>
>> Signed-off-by: Thor Thayer 

[...]


-- 
Best regards,
Marek Vasut


  1   2   3   4   5   6   7   8   9   10   >