| Message ID | cover.1539779579.git.leonard.crestez@nxp.com |
|---|---|
| Headers | show |
| Series | Port mxs-dcp to imx6ull and imx6sll | expand |
Hi Leonard, On Wed, Oct 17, 2018 at 9:38 AM Leonard Crestez <leonard.crestez@nxp.com> wrote: > > On 6ull and 6sll the DCP block has a clock which needs to be explicitly > enabled. > > Add minimal handling for this at probe/remove time. > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > --- Please always explain what changed from the previous patch version. > + /* Restart the DCP block. */ > + ret = stmp_reset_block(sdcp->base); > + if (ret) { > + dev_err(dev, "Failed reset\n"); > + goto err_disable_unprepare_clk; > + } This seems like an unrelated change that should be in a separate patch. Also, this was not present in v3, so it is better to explain that this has been introduced in v4.
On Wed, 2018-10-17 at 09:48 -0300, Fabio Estevam wrote: > On Wed, Oct 17, 2018 at 9:38 AM Leonard Crestez <leonard.crestez@nxp.com> wrote: > > > > On 6ull and 6sll the DCP block has a clock which needs to be explicitly > > enabled. > > > > Add minimal handling for this at probe/remove time. > > > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > > Please always explain what changed from the previous patch version. There is a changelog in the cover letter. > > + /* Restart the DCP block. */ > > + ret = stmp_reset_block(sdcp->base); > > + if (ret) { > > + dev_err(dev, "Failed reset\n"); > > + goto err_disable_unprepare_clk; > > + } > > This seems like an unrelated change that should be in a separate patch. > > Also, this was not present in v3, so it is better to explain that this > has been introduced in v4. This only looks slightly odd in git diff but it's not unrelated. I placed clk get/prepare/enable just before stmp_reset_block and made stmp_reset_block print a message and goto err_disable_unprepare_clk on failure instead of just "if (ret) return ret;"
On Wed, Oct 17, 2018 at 9:59 AM Leonard Crestez <leonard.crestez@nxp.com> wrote: > There is a changelog in the cover letter. It did not show up: https://lkml.org/lkml/2018/10/17/673 > This only looks slightly odd in git diff but it's not unrelated. > > I placed clk get/prepare/enable just before stmp_reset_block and made > stmp_reset_block print a message and goto err_disable_unprepare_clk on > failure instead of just "if (ret) return ret;" Got it! Then it looks fine, thanks: Reviewed-by: Fabio Estevam <festevam@gmail.com>
On Wed, Oct 17, 2018 at 12:37:54PM +0000, Leonard Crestez wrote: > The DCP block on 6ull has no major differences other than requiring > explicit clock enabling. > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > Reviewed-by: Fabio Estevam <festevam@gmail.com> Applied, thanks.
On Wed, Oct 17, 2018 at 12:37:55PM +0000, Leonard Crestez wrote: > This block is present in 6sl, 6sll and 6ull so it should be enabled in > the default imx kernel config. > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > Reviewed-by: Fabio Estevam <festevam@gmail.com> Applied, thanks.
On 10/17/2018 4:02 PM, Fabio Estevam wrote: > On Wed, Oct 17, 2018 at 9:59 AM Leonard Crestez <leonard.crestez@nxp.com> wrote: > >> There is a changelog in the cover letter. > > It did not show up: > https://lkml.org/lkml/2018/10/17/673 Sorry, I forgot to copy the changelog for v4. The only change is the one we discussed earlier in this thread. >> This only looks slightly odd in git diff but it's not unrelated. >> >> I placed clk get/prepare/enable just before stmp_reset_block and made >> stmp_reset_block print a message and goto err_disable_unprepare_clk on >> failure instead of just "if (ret) return ret;" > > Got it! Then it looks fine, thanks: > > Reviewed-by: Fabio Estevam <festevam@gmail.com> Thanks for looking at this series. Shawn already applied patches 3/4, I guess parts 1/2 should go through the crypto tree? I don't know how crypto treats patches in the merge window, I can resend with all the ack/review tags after rc1 is out.
The DCP block is present on 6sll and 6ull but not enabled. The hardware is mostly compatible with 6sl, the only important difference is that explicit clock enabling is required. There were several issues with the functionality of this driver (it didn't even probe properly) but they are fixed in cryptodev/master by this series: https://lore.kernel.org/patchwork/cover/994874/ Leonard Crestez (4): dt-bindings: crypto: Mention clocks for mxs-dcp crypto: mxs-dcp - Add support for dcp clk ARM: dts: imx6ull: Add dcp node ARM: imx_v6_v7_defconfig: Enable CRYPTO_DEV_MXS_DCP .../devicetree/bindings/crypto/fsl-dcp.txt | 2 ++ arch/arm/boot/dts/imx6ull.dtsi | 10 +++++++ arch/arm/configs/imx_v6_v7_defconfig | 1 + drivers/crypto/mxs-dcp.c | 28 +++++++++++++++++-- 4 files changed, 38 insertions(+), 3 deletions(-)