lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sun, 07 Oct 2018 19:27:50 -0700
From:   Stephen Boyd <sboyd@...nel.org>
To:     Jolly Shah <jolly.shah@...inx.com>, arm@...nel.org,
        linux-clk@...r.kernel.org, michal.simek@...inx.com,
        mturquette@...libre.com, olof@...om.net, sboyd@...eaurora.org
Cc:     rajanv@...inx.com, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, Jolly Shah <jolly.shah@...inx.com>,
        Rajan Vaja <rajan.vaja@...inx.com>,
        Tejas Patel <tejasp@...inx.com>,
        Shubhrajyoti Datta <shubhrajyoti.datta@...inx.com>,
        Jolly Shah <jollys@...inx.com>
Subject: Re: [PATCH v5 4/4] drivers: clk: Add ZynqMP clock driver

Quoting Jolly Shah (2018-09-28 15:18:00)
> diff --git a/drivers/clk/zynqmp/clkc.c b/drivers/clk/zynqmp/clkc.c
> new file mode 100644
> index 0000000..1b07d77
> --- /dev/null
> +++ b/drivers/clk/zynqmp/clkc.c
> @@ -0,0 +1,716 @@
[...]
> + * @type:      Clock type: CLK_TYPE_OUTPUT or CLK_TYPE_EXTERNAL
> + *
> + * Return: 0 on success else error code
> + */
> +static int zynqmp_get_clock_type(u32 clk_id, u32 *type)
> +{
> +       int ret;
> +
> +       ret = zynqmp_is_valid_clock(clk_id);
> +       if (ret == 1) {
> +               *type = clock[clk_id].type;
> +               return 0;
> +       }
> +
> +       return ret == 0 ? -EINVAL : ret;
> +}
> +
> +/**
> + * zynqmp_pm_clock_get_num_clocks() - Get number of clocks in system
> + * @nclocks:   Number of clocks in system/board.
> + *
> + * Call firmware API to get number of clocks.
> + *
> + * Return: 0 on success else error code.
> + */
> +static int zynqmp_pm_clock_get_num_clocks(u32 *nclocks)
> +{
> +       struct zynqmp_pm_query_data qdata = {0};
> +       __le32 ret_payload[PAYLOAD_ARG_CNT];

I asked about endianess but this isn't the right fix. Just marking
something as __le32 but then not using that type and just passing it to
some function that takes a u32 pointer doesn't work. So is the firmware
side always little endian? If so, maybe the ioctls can do the byte swap
and then the kernel API can be native CPU endian while the firmware
layer can be aware that things may need swapping. I'm suggesting that
this type is just u32 and then the eemi_ops functions accept u32 and do
the swapping themselves.

If you put back u32 here and elsewhere in this patch you can have my

Reviewed-by: Stephen Boyd <sboyd@...nel.org>

The fix to the underlying ops for endian correctness can come later, so
don't feel like it needs to be fixed right now.

Powered by blists - more mailing lists