| 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 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <54d3d1b4-29de-4d18-a39e-bf74a5c61509@linaro.org>
Date: Sat, 12 Nov 2022 10:46:53 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Johan Hovold <johan+linaro@...nel.org>,
Vinod Koul <vkoul@...nel.org>
Cc: Andy Gross <agross@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
linux-arm-msm@...r.kernel.org, linux-phy@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 18/22] phy: qcom-qmp-combo: merge driver data
On 11/11/2022 11:56, Johan Hovold wrote:
> The QMP combo driver manages a single PHY (even if it provides two
> interfaces for USB and DP, respectively) so merge the old qcom_qmp and
> qmp_phy structures and drop the PHY array.
>
> Signed-off-by: Johan Hovold <johan+linaro@...nel.org>
> ---
> drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 690 ++++++++++------------
> 1 file changed, 313 insertions(+), 377 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 249912b75964..43b7ea5d2edc 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -796,7 +796,7 @@ static const u8 qmp_dp_v5_voltage_swing_hbr_rbr[4][4] = {
> { 0x3f, 0xff, 0xff, 0xff }
> };
>
> -struct qmp_phy;
> +struct qmp_combo;
>
> struct qmp_phy_cfg {
> int lanes;
> @@ -835,10 +835,10 @@ struct qmp_phy_cfg {
> const u8 (*pre_emphasis_hbr3_hbr2)[4][4];
>
> /* DP PHY callbacks */
> - int (*configure_dp_phy)(struct qmp_phy *qphy);
> - void (*configure_dp_tx)(struct qmp_phy *qphy);
> - int (*calibrate_dp_phy)(struct qmp_phy *qphy);
> - void (*dp_aux_init)(struct qmp_phy *qphy);
> + int (*configure_dp_phy)(struct qmp_combo *qmp);
> + void (*configure_dp_tx)(struct qmp_combo *qmp);
> + int (*calibrate_dp_phy)(struct qmp_combo *qmp);
> + void (*dp_aux_init)(struct qmp_combo *qmp);
>
> /* clock ids to be requested */
> const char * const *clk_list;
> @@ -861,29 +861,19 @@ struct qmp_phy_cfg {
>
> };
>
> -/**
> - * struct qmp_phy - per-lane phy descriptor
> - *
> - * @phy: generic phy
> - * @cfg: phy specific configuration
> - * @serdes: iomapped memory space for phy's serdes (i.e. PLL)
> - * @tx: iomapped memory space for lane's tx
> - * @rx: iomapped memory space for lane's rx
> - * @pcs: iomapped memory space for lane's pcs
> - * @tx2: iomapped memory space for second lane's tx (in dual lane PHYs)
> - * @rx2: iomapped memory space for second lane's rx (in dual lane PHYs)
> - * @pcs_misc: iomapped memory space for lane's pcs_misc
> - * @pcs_usb: iomapped memory space for lane's pcs_usb
> - * @pipe_clk: pipe clock
> - * @qmp: QMP phy to which this lane belongs
> - * @mode: current PHY mode
> - * @dp_aux_cfg: Display port aux config
> - * @dp_opts: Display port optional config
> - * @dp_clks: Display port clocks
> - */
> -struct qmp_phy {
> - struct phy *phy;
> +struct qmp_phy_dp_clks {
> + struct qmp_combo *qmp;
> + struct clk_hw dp_link_hw;
> + struct clk_hw dp_pixel_hw;
> +};
> +
It would make sense to keep the kerneldoc here.
> +struct qmp_combo {
> + struct device *dev;
> +
> const struct qmp_phy_cfg *cfg;
> +
> + void __iomem *dp_com;
> +
> void __iomem *serdes;
> void __iomem *tx;
> void __iomem *rx;
> @@ -899,59 +889,33 @@ struct qmp_phy {
> void __iomem *dp_pcs;
>
> struct clk *pipe_clk;
> - struct qcom_qmp *qmp;
> - enum phy_mode mode;
> - unsigned int dp_aux_cfg;
> - struct phy_configure_opts_dp dp_opts;
> - struct qmp_phy_dp_clks *dp_clks;
> -};
> -
> -struct qmp_phy_dp_clks {
> - struct qmp_phy *qphy;
> - struct clk_hw dp_link_hw;
> - struct clk_hw dp_pixel_hw;
> -};
> -
> -/**
> - * struct qcom_qmp - structure holding QMP phy block attributes
> - *
> - * @dev: device
> - * @dp_com: iomapped memory space for phy's dp_com control block
> - *
> - * @clks: array of clocks required by phy
> - * @resets: array of resets required by phy
> - * @vregs: regulator supplies bulk data
> - *
> - * @phys: array of per-lane phy descriptors
> - * @phy_mutex: mutex lock for PHY common block initialization
> - * @init_count: phy common block initialization count
> - */
> -struct qcom_qmp {
> - struct device *dev;
> - void __iomem *dp_com;
> -
> struct clk_bulk_data *clks;
> struct reset_control_bulk_data *resets;
> struct regulator_bulk_data *vregs;
>
> - struct qmp_phy **phys;
> - struct qmp_phy *usb_phy;
> -
> struct mutex phy_mutex;
> int init_count;
> +
> + struct phy *usb_phy;
> + enum phy_mode mode;
> +
> + struct phy *dp_phy;
> + unsigned int dp_aux_cfg;
> + struct phy_configure_opts_dp dp_opts;
> + struct qmp_phy_dp_clks *dp_clks;
> };
>
> -static void qcom_qmp_v3_phy_dp_aux_init(struct qmp_phy *qphy);
> -static void qcom_qmp_v3_phy_configure_dp_tx(struct qmp_phy *qphy);
> -static int qcom_qmp_v3_phy_configure_dp_phy(struct qmp_phy *qphy);
> -static int qcom_qmp_v3_dp_phy_calibrate(struct qmp_phy *qphy);
> +static void qcom_qmp_v3_phy_dp_aux_init(struct qmp_combo *qmp);
> +static void qcom_qmp_v3_phy_configure_dp_tx(struct qmp_combo *qmp);
> +static int qcom_qmp_v3_phy_configure_dp_phy(struct qmp_combo *qmp);
> +static int qcom_qmp_v3_dp_phy_calibrate(struct qmp_combo *qmp);
>
> -static void qcom_qmp_v4_phy_dp_aux_init(struct qmp_phy *qphy);
> -static void qcom_qmp_v4_phy_configure_dp_tx(struct qmp_phy *qphy);
> -static int qcom_qmp_v4_phy_configure_dp_phy(struct qmp_phy *qphy);
> -static int qcom_qmp_v4_dp_phy_calibrate(struct qmp_phy *qphy);
> +static void qcom_qmp_v4_phy_dp_aux_init(struct qmp_combo *qmp);
> +static void qcom_qmp_v4_phy_configure_dp_tx(struct qmp_combo *qmp);
> +static int qcom_qmp_v4_phy_configure_dp_phy(struct qmp_combo *qmp);
> +static int qcom_qmp_v4_dp_phy_calibrate(struct qmp_combo *qmp);
>
> -static int qcom_qmp_v5_phy_configure_dp_phy(struct qmp_phy *qphy);
> +static int qcom_qmp_v5_phy_configure_dp_phy(struct qmp_combo *qmp);
As you are doing the cleanup anyway, would it make sense to move these
functions up to be able to drop these prototypes?
>
> static inline void qphy_setbits(void __iomem *base, u32 offset, u32 val)
> {
> @@ -1265,11 +1229,11 @@ static void qmp_combo_configure(void __iomem *base,
The rest LGTM
--
With best wishes
Dmitry
Powered by blists - more mailing lists