Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1547888

Re: [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy

Show all headers | View raw

Hi Jaehoon,

On Wed, Dec 28, 2016 at 8:19 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi Vivek,
>
> On 12/27/2016 02:53 PM, Vivek Gautam wrote:
>> Hi Jaehoon,
>>
>>
>> On Mon, Dec 26, 2016 at 10:50 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>> This patch supports to use Generic Phy framework for Exynos PCIe phy.
>>> When Exynos that supported the pcie want to use the PCIe,
>>> it needs to control the phy resgister.
>>> But it should be more complex to control in their own PCIe device drivers.
>>>
>>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>>> ---
>>>  drivers/phy/Kconfig           |   9 ++
>>>  drivers/phy/Makefile          |   1 +
>>>  drivers/phy/phy-exynos-pcie.c | 227 ++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 237 insertions(+)
>>>  create mode 100644 drivers/phy/phy-exynos-pcie.c
>>>
>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>> index fe00f91..94b0433 100644
>>> --- a/drivers/phy/Kconfig
>>> +++ b/drivers/phy/Kconfig
>>> @@ -341,6 +341,15 @@ config PHY_EXYNOS5_USBDRD
>>>           This driver provides PHY interface for USB 3.0 DRD controller
>>>           present on Exynos5 SoC series.
>>>
>>> +config PHY_EXYNOS_PCIE
>>> +       bool "Exynos PCIe PHY driver"
>>
>> Is there a reason for this not being 'tristate' ?
>
> Will change.

I notice that PCI_EXYNOS5433 is bool as well.
If the host has to be 'bool' then it makes sense to have phy
also bool as well. But if PCI_EXYNOS5433 can be made
tristate, then this also changes to tristate.

>
>>
>>> +       depends on ARCH_EXYNOS && OF
>>> +       depends on PCI_EXYNOS5433
>>> +       select GENERIC_PHY
>>> +       help
>>> +         Enable PCIe PHY support for Exynos SoC series.
>>
>> If this driver is for Exynos5433, then same should come in this help
>> text as well.
>
> will support the other exynos series.
> I'm working on refactoring exynos5440 with PHY generic Framework.
> Then this drive is not for only Exnyos5433. how about?

Ok, it's good then. My only concern is 'depends on PCI_EXYNOS5433'
makes it look like it is for EXYNOS5433. I am fine if that changes as well.

[...]

>>> +
>>> +#define PCIE_EXYNOS5433_PMU_PHY_OFFSET         0x730
>>> +#define PCIE_PHY_OFFSET(x)             ((x) * 0x4)
>>> +
>>> +/* Sysreg Fsys register offset and bit for Exynos5433 */
>>> +#define PCIE_PHY_MAC_RESET             0x208
>>> +#define PCIE_MAC_RESET_MASK            0xFF
>>> +#define PCIE_MAC_RESET                 BIT(4)
>>> +#define PCIE_L1SUB_CM_CON              0x1010
>>> +#define PCIE_REFCLK_GATING_EN          BIT(0)
>>> +#define PCIE_PHY_COMMON_RESET          0x1020
>>> +#define PCIE_PHY_RESET                 BIT(0)
>>> +#define PCIE_PHY_GLOBAL_RESET          0x1040
>>> +#define PCIE_GLOBAL_RESET              BIT(0)
>>> +#define PCIE_REFCLK                    BIT(1)
>>> +#define PCIE_REFCLK_MASK               0x16
>>> +#define PCIE_APP_REQ_EXIT_L1_MODE      BIT(5)
>>> +
>>> +enum exynos_pcie_phy_data_type {
>>> +       PCIE_PHY_TYPE_EXYNOS5433,
>>> +};
>>> +
>>> +struct exynos_pcie_phy_data {
>>> +       enum exynos_pcie_phy_data_type  ctrl_type;
>>
>> Why do we need this controller type ?
>> If there are changes in the IP between different version,
>> then you can as well use different compatibles.
>
> Do you mean is the using "of_device_is_compatible()"?

I meant that multiple compatible strings can be added based on the
IP versions. And any IP specific data can be put in the .data field
of  'of_device_id' structure.
If there's more to differentiate between the IP versions at runtime,
you can use of_device_is_compatible().

[...]



-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[RFC PATCH 0/6] Support the PCIe for TM2(exynos5433) Jaehoon Chung <jh80.chung@samsung.com> - 2016-12-26 06:30 +0100
  [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy Jaehoon Chung <jh80.chung@samsung.com> - 2016-12-26 06:30 +0100
    Re: [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy Vivek Gautam <vivek.gautam@codeaurora.org> - 2016-12-27 07:00 +0100
      Re: [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy Jaehoon Chung <jh80.chung@samsung.com> - 2016-12-28 04:00 +0100
        Re: [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy Vivek Gautam <vivek.gautam@codeaurora.org> - 2016-12-28 10:00 +0100
          Re: [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy Jaehoon Chung <jh80.chung@samsung.com> - 2016-12-28 10:50 +0100
            Re: [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy Vivek Gautam <vivek.gautam@codeaurora.org> - 2016-12-28 11:20 +0100
  [RFC PATCH 4/6] PCI: exynos5433: Add new exynos pci host controller  for Exynos5433 Jaehoon Chung <jh80.chung@samsung.com> - 2016-12-26 06:30 +0100
    Re: [RFC PATCH 4/6] PCI: exynos5433: Add new exynos pci host  controller for Exynos5433 Joao Pinto <Joao.Pinto@synopsys.com> - 2016-12-26 13:00 +0100
      Re: [RFC PATCH 4/6] PCI: exynos5433: Add new exynos pci host  controller for Exynos5433 Jaehoon Chung <jh80.chung@samsung.com> - 2016-12-27 02:50 +0100

csiph-web