lkml.org 
[lkml]   [2019]   [Feb]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 9/9] phy: qcom-ufs: Refactor all init steps into phy_poweron
From
Date
Quoting Evan Green (2019-01-23 14:11:37)
> The phy code was using implicit sequencing between the PHY driver
> and the UFS driver to implement certain hardware requirements.
> Specifically, the PHY reset register in the UFS controller needs
> to be deasserted before serdes start occurs in the PHY.
>
> Before this change, the code was doing this by utilizing the two
> phy callbacks, phy_init and phy_poweron, as "init step 1" and

Nitpick: Can you please indicate functions with () and variables with
''? So write phy_init() and phy_poweron(), etc.

> "init step 2", where the UFS driver would deassert reset between
> these two steps.
>
> This makes it challenging to power off the regulators in suspend,
> as regulators are initialized in init, not in poweron, but only
> poweroff is called during suspend, not exit.
>
> Consolidate the initialization code into phy_poweron, and utilize
> the reset controller exported from the UFS driver to explicitly
> perform all the steps needed to initialize the PHY.

Also mention that a new callback is introduced, 'calibrate', that
>
> Signed-off-by: Evan Green <evgreen@chromium.org>

> diff --git a/drivers/phy/qualcomm/phy-qcom-ufs-i.h b/drivers/phy/qualcomm/phy-qcom-ufs-i.h
> index f798fb64de94e..109ddd67be829 100644
> --- a/drivers/phy/qualcomm/phy-qcom-ufs-i.h
> +++ b/drivers/phy/qualcomm/phy-qcom-ufs-i.h
> @@ -19,6 +19,7 @@
> #include <linux/clk.h>
> #include <linux/phy/phy.h>
> #include <linux/regulator/consumer.h>
> +#include <linux/reset.h>

Just forward declare struct reset_control instead of including this.

> #include <linux/slab.h>
> #include <linux/platform_device.h>
> #include <linux/io.h>
> @@ -96,11 +97,10 @@ struct ufs_qcom_phy {
> char name[UFS_QCOM_PHY_NAME_LEN];
> struct ufs_qcom_phy_calibration *cached_regs;
> int cached_regs_table_size;
> - bool is_powered_on;
> - bool is_started;
> struct ufs_qcom_phy_specific_ops *phy_spec_ops;
>
> enum phy_mode mode;
> + struct reset_control *ufs_reset;
> };
>
> /**

For some reason I get the feeling that this patch should be combined
with something else from the controller. Does this complete the
conversion but the patches before this one sort of wreck the state of
reset and init/poweron phases so that they can't stand on their own?
Maybe if the reset was introduced, and then a patch to get the resets
was put in place, and then a final patch to rewrite the phy and
controller at the same time would make more sense to read.

\
 
 \ /
  Last update: 2019-02-01 19:44    [W:0.101 / U:4.228 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site