Add Intel ComboPhy driver
Subsystems (2)
v7 cover Add Intel ComboPhy driver
This patch series adds Intel ComboPhy driver, respective yaml schemas
Changes on v7:
As per System control driver maintainer's inputs remove
fwnode_to_regmap() definition and use device_node_get_regmap()
Changes on v6:
Rebase patches on the latest maintainer's branch
https://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git/?h=phy-for-5.7
Dilip Kota (3):
dt-bindings: phy: Add PHY_TYPE_XPCS definition
dt-bindings: phy: Add YAML schemas for Intel ComboPhy
phy: intel: Add driver support for ComboPhy
.../devicetree/bindings/phy/intel,combo-phy.yaml | 101 ++++ drivers/phy/intel/Kconfig | 14 + drivers/phy/intel/Makefile | 1 + drivers/phy/intel/phy-intel-combo.c | 627 +++++++++++++++++++++ include/dt-bindings/phy/phy.h | 1 + 5 files changed, 744 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/intel,combo-phy.yaml create mode 100644 drivers/phy/intel/phy-intel-combo.c
Signature · 1 line
2.11.0
PATCH v7 1/3 dt-bindings: phy: Add PHY_TYPE_XPCS definition
Add definition for Ethernet PCS phy type.
---
Changes on v7:
No Change
Changes on v6:
Add Acked-by: Rob Herring <[email protected]>
include/dt-bindings/phy/phy.h | 1 + 1 file changed, 1 insertion(+)
include/dt-bindings/phy/phy.h +1 -0
@@ -17,5 +17,6 @@#define PHY_TYPE_USB3 4#define PHY_TYPE_UFS 5#define PHY_TYPE_DP 6 +#define PHY_TYPE_XPCS 7#endif /* _DT_BINDINGS_PHY */
Signature · 1 line
2.11.0
PATCH v7 2/3 dt-bindings: phy: Add YAML schemas for Intel ComboPhy
In reply to Dilip KotaComboPhy subsystem provides PHY support to various controllers, viz. PCIe, SATA and EMAC. Adding YAML schemas for the same.
---
Changes on v7:
No Change.
Changes on v6:
Add Reviewed-by: Rob Herring <[email protected]>
.../devicetree/bindings/phy/intel,combo-phy.yaml | 101 +++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/intel,combo-phy.yaml
Documentation/devicetree/bindings/phy/intel,combo-phy.yaml new file +101 -0
@@ -0,0 +1,101 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/phy/intel,combo-phy.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Intel ComboPhy Subsystem + +maintainers: + - Dilip Kota <[email protected]> + +description: | + Intel Combophy subsystem supports PHYs for PCIe, EMAC and SATA + controllers. A single Combophy provides two PHY instances. + +properties: + $nodename: + pattern: "combophy(@.*|-[0-9a-f])*$" + + compatible: + items: + - const: intel,combophy-lgm + - const: intel,combo-phy + + clocks: + maxItems: 1 + + reg: + items: + - description: ComboPhy core registers + - description: PCIe app core control registers + + reg-names: + items: + - const: core + - const: app + + resets: + maxItems: 4 + + reset-names: + items: + - const: phy + - const: core + - const: iphy0 + - const: iphy1 + + intel,syscfg: + $ref: /schemas/types.yaml#/definitions/phandle-array + description: Chip configuration registers handle and ComboPhy instance id + + intel,hsio: + $ref: /schemas/types.yaml#/definitions/phandle-array + description: HSIO registers handle and ComboPhy instance id on NOC + + intel,aggregation: + type: boolean + description: | + Specify the flag to configure ComboPHY in dual lane mode. + + intel,phy-mode: + $ref: /schemas/types.yaml#/definitions/uint32 + description: | + Mode of the two phys in ComboPhy. + See dt-bindings/phy/phy.h for values. + + "#phy-cells": + const: 1 + +required: + - compatible + - clocks + - reg + - reg-names + - intel,syscfg + - intel,hsio + - intel,phy-mode + - "#phy-cells" + +additionalProperties: false + +examples: + - | + #include <dt-bindings/phy/phy.h> + combophy@d0a00000 { + compatible = "intel,combophy-lgm", "intel,combo-phy"; + clocks = <&cgu0 1>; + #phy-cells = <1>; + reg = <0xd0a00000 0x40000>, + <0xd0a40000 0x1000>; + reg-names = "core", "app"; + resets = <&rcu0 0x50 6>, + <&rcu0 0x50 17>, + <&rcu0 0x50 23>, + <&rcu0 0x50 24>; + reset-names = "phy", "core", "iphy0", "iphy1"; + intel,syscfg = <&sysconf 0>; + intel,hsio = <&hsiol 0>; + intel,phy-mode = <PHY_TYPE_PCIE>; + intel,aggregation; + };
Signature · 1 line
2.11.0
PATCH v7 3/3 phy: intel: Add driver support for ComboPhy
In reply to Dilip KotaComboPhy subsystem provides PHYs for various
controllers like PCIe, SATA and EMAC.
---
Changes on v7:
Use device_node_to_regmap instead of fwnode_to_regmap
Changes on v6:
No changes
Changes on v5:
Add changes as per inputs from Andy and Rob:
DT node uses phy-mode values as defined in "include/dt-bindings/phy/phy.h",
add changes to handle it.
ComboPhy no longer has children nodes, and children node properties(reset)
moved to parent node, so do the code changes accordingly.
Add _xlate() function to pass the appropriate phy handle.
Fix couple of nitpicks.
Changes on v4:
Address review comments
Remove dependency on OF config
Update copyright to 2019-2020
Define register macro PAD_DIS_CFG instead of const variable inside function. Improve the error prints, and error returns.
Call put_device(dev), for get_dev_from_fwnode()
Move platform_set_drvdata() at the end of the probe().
Correct alignment in phy_ops intel_cbphy_ops.
Correct commented lines with proper vocabulary and punctuation. Add/remove commas for the required constant arrays and enums. Remove in driver:
linux/kernel.h, not required
macros: PCIE_PHY_MPLLA_CTRL, PCIE_PHY_MPLLB_CTRL
temp variable u32 prop;
Change function names:
intel_cbphy_iphy_dt_parse() -> intel_cbphy_iphy_fwnode_parse()
intel_cbphy_dt_sanity_check() -> intel_cbphy_sanity_check()
intel_cbphy_dt_parse() -> intel_cbphy_fwnode_parse()Changes on v3:
Remove intel_iphy_names
Remove struct phy in struct intel_cbphy_iphy
Imporve if conditions logic
Use fwnode_to_regmap()
Call devm_of_platform_populate() to populate child nodes Fix reset sequence during phy_init
Add SoC specific compatible "intel,combophy-lgm"
Add description for enums
Remove default case in switch {} intel_cbphy_set_mode() as it
never happens.
Use mutex_lock to synchronise combophy initialization across
two phys.
Change init_cnt to u32 datatype as it is within mutex lock. Correct error handling of
fwnode_property_read_u32_array(fwnode, "intel,phy-mode", ...)
drivers/phy/intel/Kconfig | 14 + drivers/phy/intel/Makefile | 1 + drivers/phy/intel/phy-intel-combo.c | 627 ++++++++++++++++++++++++++++++++++++ 3 files changed, 642 insertions(+) create mode 100644 drivers/phy/intel/phy-intel-combo.c
drivers/phy/intel/Kconfig +14 -0
@@ -2,6 +2,20 @@ # # Phy drivers for Intel Lightning Mountain(LGM) platform # +config PHY_INTEL_COMBO + bool "Intel ComboPHY driver" + depends on X86 || COMPILE_TEST + depends on OF && HAS_IOMEM + select MFD_SYSCON + select GENERIC_PHY + select REGMAP + help + Enable this to support Intel ComboPhy. + + This driver configures ComboPhy subsystem on Intel gateway + chipsets which provides PHYs for various controllers, EMAC, + SATA and PCIe. + config PHY_INTEL_EMMC tristate "Intel EMMC PHY driver" select GENERIC_PHY
drivers/phy/intel/Makefile +1 -0
@@ -1,2 +1,3 @@ # SPDX-License-Identifier: GPL-2.0 +obj-$(CONFIG_PHY_INTEL_COMBO) += phy-intel-combo.o obj-$(CONFIG_PHY_INTEL_EMMC) += phy-intel-emmc.o
drivers/phy/intel/phy-intel-combo.c new file +627 -0
627 lines not on this page · raw message
Signature · 1 line
2.11.0
Vinod Koul
maintainerRe: [PATCH v7 3/3] phy: intel: Add driver support for ComboPhy
drivers/phy/intel/phy-intel-combo.c line 50
Quoted 4 lines · Dilip Kota
drivers/phy/intel/phy-intel-combo.c new file +4
-0+enum { +PHY_0 , +PHY_1 , +PHY_MAX_NUM
PHY_MAX_NUM = PHY_1?
drivers/phy/intel/phy-intel-combo.c line 138
Quoted 9 lines
drivers/phy/intel/phy-intel-combo.c new file +9
-0+static inline void combo_phy_w32_off_mask (void __iomem *base,unsigned int reg, +u32 mask,u32 val) +{ +u32 reg_val; + + reg_val =readl (base + reg); + reg_val &= ~mask; + reg_val |=FIELD_PREP (mask, val); +writel (reg_val, base + reg);
bypassing regmap here... why?
Quoted 10 lines
drivers/phy/intel/phy-intel-combo.c new file +10
-0+static int intel_cbphy_calibrate (struct phy *phy) +{ +struct intel_cbphy_iphy *iphy =phy_get_drvdata (phy); +struct intel_combo_phy *cbphy = iphy->parent; +void __iomem *cr_base = cbphy->cr_base; +int val, ret, id; + +if (cbphy->phy_mode !=PHY_XPCS_MODE ) +return 0 ; +
drivers/phy/intel/phy-intel-combo.c line 393
Quoted 10 lines
drivers/phy/intel/phy-intel-combo.c new file +10
-0+ id =PHY_ID (iphy); + +/* trigger auto RX adaptation */ +combo_phy_w32_off_mask (cr_base,CR_ADDR (PCS_XF_ATE_OVRD_IN_2 , id), +ADAPT_REQ_MSK ,3 ); +/* Wait RX adaptation to finish */ + ret =readl_poll_timeout (cr_base +CR_ADDR (PCS_XF_RX_ADAPT_ACK , id), + val, val &RX_ADAPT_ACK_BIT ,10 ,5000 ); +if (ret) +dev_err (cbphy->dev,"RX Adaptation failed!\n" );
you want to continue her and not return error?
Signature · 1 line
~Vinod
Re: [PATCH v7 3/3] phy: intel: Add driver support for ComboPhy
Quoted 7 lines · Vinod Koul
drivers/phy/intel/phy-intel-combo.c line 50
Quoted 4 lines · Dilip Kota
drivers/phy/intel/phy-intel-combo.c new file +4
-0+enum { +PHY_0 , +PHY_1 , +PHY_MAX_NUM
PHY_MAX_NUM=PHY_1?
Driver is using it for no. of PHYs/maximum PHY id.
Code snippets:
struct intel_combo_phy {
...
struct reset_control *phy_rst; struct reset_control *core_rst; struct intel_cbphy_iphy iphy[PHY_MAX_NUM];
...
}
....
static int intel_cbphy_create(struct intel_combo_phy *cbphy) {
struct phy_provider *phy_provider;
struct device *dev = cbphy->dev;
struct intel_cbphy_iphy *iphy;
int i;
for (i = 0; i < PHY_MAX_NUM; i++) {
...
Quoted 11 lines
drivers/phy/intel/phy-intel-combo.c line 138
Quoted 10 lines
drivers/phy/intel/phy-intel-combo.c new file +9
-0+static inline void combo_phy_w32_off_mask (void __iomem *base,unsigned int reg, +u32 mask,u32 val) +{ +u32 reg_val; + + reg_val =readl (base + reg); + reg_val &= ~mask; + reg_val |=FIELD_PREP (mask, val); +writel (reg_val, base + reg);bypassing regmap here... why?
It is not regmap address, one of the below two addresses are passed to this function.
struct intel_combo_phy {
...
void __iomem *app_base;
void __iomem *cr_base;
...
}
Quoted 14 lines
Quoted 14 lines
drivers/phy/intel/phy-intel-combo.c new file +13
-0+static int intel_cbphy_calibrate (struct phy *phy) +{ +struct intel_cbphy_iphy *iphy =phy_get_drvdata (phy); +struct intel_combo_phy *cbphy = iphy->parent; +void __iomem *cr_base = cbphy->cr_base; +int val, ret, id; + +if (cbphy->phy_mode !=PHY_XPCS_MODE ) +return 0 ; + + id =PHY_ID (iphy); + +/* trigger auto RX adaptation */
Quoted 8 lines
drivers/phy/intel/phy-intel-combo.c line 393
Quoted 7 lines
drivers/phy/intel/phy-intel-combo.c new file +7
-0+combo_phy_w32_off_mask (cr_base,CR_ADDR (PCS_XF_ATE_OVRD_IN_2 , id), +ADAPT_REQ_MSK ,3 ); +/* Wait RX adaptation to finish */ + ret =readl_poll_timeout (cr_base +CR_ADDR (PCS_XF_RX_ADAPT_ACK , id), + val, val &RX_ADAPT_ACK_BIT ,10 ,5000 ); +if (ret) +dev_err (cbphy->dev,"RX Adaptation failed!\n" );you want to continue her and not return error?
Next step is stopping the Adaptation, it should be done in both error and success case.
Regards,
Dilip
Vinod Koul
maintainerRe: [PATCH v7 3/3] phy: intel: Add driver support for ComboPhy
Quoted 10 lines · Dilip Kota
Quoted 7 lines · Vinod Koul
drivers/phy/intel/phy-intel-combo.c line 50
Quoted 4 lines · Dilip Kota
drivers/phy/intel/phy-intel-combo.c new file +4
-0+enum { +PHY_0 , +PHY_1 , +PHY_MAX_NUM
PHY_MAX_NUM=PHY_1?Driver is using it for no. of PHYs/maximum PHY id.
Ok
Quoted 10 lines
Quoted 10 lines
drivers/phy/intel/phy-intel-combo.c line 138
Quoted 9 lines
drivers/phy/intel/phy-intel-combo.c new file +9
-0+static inline void combo_phy_w32_off_mask (void __iomem *base,unsigned int reg, +u32 mask,u32 val) +{ +u32 reg_val; + + reg_val =readl (base + reg); + reg_val &= ~mask; + reg_val |=FIELD_PREP (mask, val); +writel (reg_val, base + reg);bypassing regmap here... why?
Quoted 2 lines
It is not regmap address, one of the below two addresses are passed to this function.
okay, perhaps add a comment somewhere that regmap is not used for this base?
Quoted 30 lines
struct
intel_combo_phy{
...
void __iomem*app_base;
void __iomem*cr_base;
...
}Quoted 13 lines
Quoted 13 lines
drivers/phy/intel/phy-intel-combo.c new file +13
-0+static int intel_cbphy_calibrate (struct phy *phy) +{ +struct intel_cbphy_iphy *iphy =phy_get_drvdata (phy); +struct intel_combo_phy *cbphy = iphy->parent; +void __iomem *cr_base = cbphy->cr_base; +int val, ret, id; + +if (cbphy->phy_mode !=PHY_XPCS_MODE ) +return 0 ; + + id =PHY_ID (iphy); + +/* trigger auto RX adaptation */ Quoted 8 lines
drivers/phy/intel/phy-intel-combo.c line 393
Quoted 7 lines
drivers/phy/intel/phy-intel-combo.c new file +7
-0+combo_phy_w32_off_mask (cr_base,CR_ADDR (PCS_XF_ATE_OVRD_IN_2 , id), +ADAPT_REQ_MSK ,3 ); +/* Wait RX adaptation to finish */ + ret =readl_poll_timeout (cr_base +CR_ADDR (PCS_XF_RX_ADAPT_ACK , id), + val, val &RX_ADAPT_ACK_BIT ,10 ,5000 ); +if (ret) +dev_err (cbphy->dev,"RX Adaptation failed!\n" );you want to continue her and not return error?
Quoted 2 lines
Next step is stopping the Adaptation, it should be done in both error and success case.
Again documenting this helps, pls add some comments on this behaviour
Signature · 1 line
~Vinod
Re: [PATCH v7 3/3] phy: intel: Add driver support for ComboPhy
Quoted 13 lines · Vinod Koul
Quoted 12 lines · Dilip Kota
Quoted 11 lines · Vinod Koul
drivers/phy/intel/phy-intel-combo.c line 138
Quoted 8 lines · Dilip Kota
drivers/phy/intel/phy-intel-combo.c new file +8
-0+ u32 mask, u32 val) +{ +u32 reg_val; + + reg_val =readl (base + reg); + reg_val &= ~mask; + reg_val |=FIELD_PREP (mask, val); +writel (reg_val, base + reg);bypassing regmap here... why?
Quoted 4 lines
Quoted 2 lines
It is not regmap address, one of the below two addresses are passed to this function.
okay, perhaps add a comment somewhere that regmap is not used for this base?
I dont see a need of adding a comment, describing don't do regmap here.
Quoted 29 lines
Quoted 29 lines
struct
intel_combo_phy{
...
void __iomem*app_base;
void __iomem*cr_base;
...
}Quoted 21 lines
Quoted 10 lines
drivers/phy/intel/phy-intel-combo.c new file +10
-0+static int intel_cbphy_calibrate (struct phy *phy) +{ +struct intel_cbphy_iphy *iphy =phy_get_drvdata (phy); +struct intel_combo_phy *cbphy = iphy->parent; +void __iomem *cr_base = cbphy->cr_base; +int val, ret, id; + +if (cbphy->phy_mode !=PHY_XPCS_MODE ) +return 0 ; +drivers/phy/intel/phy-intel-combo.c line 393
Quoted 10 lines
drivers/phy/intel/phy-intel-combo.c new file +10
-0+ id =PHY_ID (iphy); + +/* trigger auto RX adaptation */ +combo_phy_w32_off_mask (cr_base,CR_ADDR (PCS_XF_ATE_OVRD_IN_2 , id), +ADAPT_REQ_MSK ,3 ); +/* Wait RX adaptation to finish */ + ret =readl_poll_timeout (cr_base +CR_ADDR (PCS_XF_RX_ADAPT_ACK , id), + val, val &RX_ADAPT_ACK_BIT ,10 ,5000 ); +if (ret) +dev_err (cbphy->dev,"RX Adaptation failed!\n" );you want to continue her and not return error?
Quoted 3 lines
Quoted 2 lines
Next step is stopping the Adaptation, it should be done in both error and success case.
Again documenting this helps, pls add some comments on this behaviour
Comments are already in place, mentioning Start and Stop of Rx Adaptation. And Stop is being is done as Start is triggered, so not needed to mention error and success.
Regards,
Dilip
Quoted 1 line
Vinod Koul
maintainerRe: [PATCH v7 3/3] phy: intel: Add driver support for ComboPhy
Quoted 15 lines · Dilip Kota
Quoted 13 lines · Vinod Koul
Quoted 12 lines · Dilip Kota
Quoted 11 lines · Vinod Koul
drivers/phy/intel/phy-intel-combo.c line 138
Quoted 8 lines · Dilip Kota
drivers/phy/intel/phy-intel-combo.c new file +8
-0+ u32 mask, u32 val) +{ +u32 reg_val; + + reg_val =readl (base + reg); + reg_val &= ~mask; + reg_val |=FIELD_PREP (mask, val); +writel (reg_val, base + reg);bypassing regmap here... why?
Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
It is not regmap address, one of the below two addresses are passed to this function.
okay, perhaps add a comment somewhere that regmap is not used for this base?
I dont see a need of adding a comment, describing don't do regmap here.
Driver uses regmap except here, which seems odd hence explanation required for this.
Quoted 29 lines
Quoted 29 lines
Quoted 29 lines
struct
intel_combo_phy{
...
void __iomem*app_base;
void __iomem*cr_base;
...
}Quoted 21 lines
Quoted 10 lines
drivers/phy/intel/phy-intel-combo.c new file +10
-0+static int intel_cbphy_calibrate (struct phy *phy) +{ +struct intel_cbphy_iphy *iphy =phy_get_drvdata (phy); +struct intel_combo_phy *cbphy = iphy->parent; +void __iomem *cr_base = cbphy->cr_base; +int val, ret, id; + +if (cbphy->phy_mode !=PHY_XPCS_MODE ) +return 0 ; +drivers/phy/intel/phy-intel-combo.c line 393
Quoted 10 lines
drivers/phy/intel/phy-intel-combo.c new file +10
-0+ id =PHY_ID (iphy); + +/* trigger auto RX adaptation */ +combo_phy_w32_off_mask (cr_base,CR_ADDR (PCS_XF_ATE_OVRD_IN_2 , id), +ADAPT_REQ_MSK ,3 ); +/* Wait RX adaptation to finish */ + ret =readl_poll_timeout (cr_base +CR_ADDR (PCS_XF_RX_ADAPT_ACK , id), + val, val &RX_ADAPT_ACK_BIT ,10 ,5000 ); +if (ret) +dev_err (cbphy->dev,"RX Adaptation failed!\n" );you want to continue her and not return error?
Quoted 6 lines
Quoted 3 lines
Quoted 2 lines
Next step is stopping the Adaptation, it should be done in both error and success case.
Again documenting this helps, pls add some comments on this behaviour
Comments are already in place, mentioning Start and Stop of Rx Adaptation. And Stop is being is done as Start is triggered, so not needed to mention error and success.
Ok
Signature · 1 line
~Vinod
Re: [PATCH v7 3/3] phy: intel: Add driver support for ComboPhy
Quoted 15 lines · Vinod Koul
Quoted 14 lines · Dilip Kota
Quoted 13 lines · Vinod Koul
Quoted 12 lines · Dilip Kota
Quoted 11 lines · Vinod Koul
drivers/phy/intel/phy-intel-combo.c line 138
Quoted 8 lines · Dilip Kota
drivers/phy/intel/phy-intel-combo.c new file +8
-0+ u32 mask, u32 val) +{ +u32 reg_val; + + reg_val =readl (base + reg); + reg_val &= ~mask; + reg_val |=FIELD_PREP (mask, val); +writel (reg_val, base + reg);bypassing regmap here... why?
Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
It is not regmap address, one of the below two addresses are passed to this function.
okay, perhaps add a comment somewhere that regmap is not used for this base?
I dont see a need of adding a comment, describing don't do regmap here.
Driver uses regmap except here, which seems odd hence explanation required for this.
During the driver Probe, the register phandles are stored in regmap datatype variables and PHY core addresses are stored in iomem datatype.
Since then, regmap access is performed for the regmap datatype variables and readl/writel access is performed on the iomem datatype variables. And nowhere in the driver iomem datatype address are converted to regmap address and performed regmap access.
Driver is not doing any 'regmap_init' on any physical address. Driver is getting the register address phandle from the device tree node and performing the regmap access. ret = fwnode_property_get_reference_args(fwnode, "intel,syscfg", NULL, 1, 0, &ref);
[...]cbphy->syscfg = device_node_to_regmap(to_of_node(ref.fwnode));
[...]
ret = fwnode_property_get_reference_args(fwnode, "intel,hsio", NULL, 1, 0, &ref); [...]
cbphy->hsiocfg = device_node_to_regmap(to_of_node(ref.fwnode));
[...]cbphy->app_base = devm_platform_ioremap_resource_byname(pdev, "app"); [...]cbphy->cr_base = devm_platform_ioremap_resource_byname(pdev, "core");
The DT parsing logic in the driver is explaining why the PHY driver should do regmap access and to whom should be done. For this reason i am a bit puzzled to what more is needed to explain in the comments and where to add it. Please let me know your view.
Regards,
Dilip
Re: [PATCH v7 3/3] phy: intel: Add driver support for ComboPhy
Hi Vinod,
Quoted 38 lines · Dilip Kota
Quoted 15 lines · Vinod Koul
Quoted 14 lines · Dilip Kota
Quoted 13 lines · Vinod Koul
Quoted 12 lines · Dilip Kota
Quoted 11 lines · Vinod Koul
Review
Quoted 8 lines · Dilip Kota
+ u32 mask, u32 val)
(quoted diff) +7
-0+{ + u32 reg_val; + + reg_val = readl(base + reg); + reg_val &= ~mask; + reg_val |= FIELD_PREP(mask, val); + writel(reg_val, base + reg);bypassing regmap here... why?
Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
It is not regmap address, one of the below two addresses are passed to this function.
okay, perhaps add a comment somewhere that regmap is not used for this base?
I dont see a need of adding a comment, describing don't do regmap here.
Driver uses regmap except here, which seems odd hence explanation required for this.
During the driver Probe, the register phandles are stored in regmap datatype variables and PHY core addresses are stored in iomem datatype.
Since then, regmap access is performed for the regmap datatype variables and readl/writel access is performed on the iomem datatype variables. And nowhere in the driver iomem datatype address are converted to regmap address and performed regmap access.Driver is not doing any '
regmap_init' on any physical address. Driver is getting the register address phandle from the device tree node and performing the regmap access. ret =fwnode_property_get_reference_args(fwnode, "intel,syscfg", NULL, 1, 0, &ref);
[...]cbphy->syscfg= device_node_to_regmap(to_of_node(ref.fwnode));[...]
ret =fwnode_property_get_reference_args(fwnode, "intel,hsio", NULL, 1, 0, &ref); [...]
cbphy->hsiocfg= device_node_to_regmap(to_of_node(ref.fwnode));
Quoted 8 lines
[...]
cbphy->app_base=devm_platform_ioremap_resource_byname(pdev, "app"); [...]cbphy->cr_base=devm_platform_ioremap_resource_byname(pdev, "core");The DT parsing logic in the driver is explaining why the PHY driver should do regmap access and to whom should be done. For this reason i am a bit puzzled to what more is needed to explain in the comments and where to add it. Please let me know your view.
Gentle Reminder!
Could you please update on this.
Regards,
Dilip
Quoted 2 lines
Regards,
Dilip
Quoted 14 lines · Dilip Kota
This patch series adds Intel ComboPhy driver, respective yaml schemas
Changes on v7:
As per System control driver maintainer's inputs remove
fwnode_to_regmap()definition and usedevice_node_get_regmap()Changes on v6:
Rebase patches on the latest maintainer's branch
https://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git/?h=phy-for-5.7
Dilip Kota (3):
dt-bindings: phy: AddPHY_TYPE_XPCSdefinition
dt-bindings: phy: Add YAML schemas for Intel ComboPhy
phy: intel: Add driver support for ComboPhy
Quoted 5 lines
.../devicetree/bindings/phy/intel,combo-phy.yaml | 101 ++++ drivers/phy/intel/Kconfig | 14 + drivers/phy/intel/Makefile | 1 + drivers/phy/intel/phy-intel-combo.c | 627 +++++++++++++++++++++ include/dt-bindings/phy/phy.h | 1 +
Why have you sent this to me?
Quoted 4 lines
5 files changed, 744 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/intel,combo-phy.yaml create mode 100644 drivers/phy/intel/phy-intel-combo.c
Signature · 4 lines
Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
Quoted 16 lines · Lee Jones
Quoted 14 lines · Dilip Kota
This patch series adds Intel ComboPhy driver, respective yaml schemas
Changes on v7:
As per System control driver maintainer's inputs remove
fwnode_to_regmap()definition and usedevice_node_get_regmap()Changes on v6:
Rebase patches on the latest maintainer's branch
https://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git/?h=phy-for-5.7
Dilip Kota (3):
dt-bindings: phy: AddPHY_TYPE_XPCSdefinition
dt-bindings: phy: Add YAML schemas for Intel ComboPhy
phy: intel: Add driver support for ComboPhy
Quoted 6 lines
Quoted 5 lines
.../devicetree/bindings/phy/intel,combo-phy.yaml | 101 ++++ drivers/phy/intel/Kconfig | 14 + drivers/phy/intel/Makefile | 1 + drivers/phy/intel/phy-intel-combo.c | 627 +++++++++++++++++++++ include/dt-bindings/phy/phy.h | 1 +Why have you sent this to me?
The main reason for this patch series is removing fwnode_to_regmap() and using device_node_get_regmap() compared to previous patch series. This has been done as per your review comments.
To keep you updated that changes are done as per your review comments, sent to you along with PHY maintainers.
Regards,
Dilip
Quoted 5 lines
Quoted 5 lines
5 files changed, 744 insertions(+)create mode 100644
Documentation/devicetree/bindings/phy/intel,combo-phy.yamlcreate mode 100644drivers/phy/intel/phy-intel-combo.c
Quoted 22 lines · Dilip Kota
Quoted 20 lines · Lee Jones
Quoted 18 lines · Dilip Kota
This patch series adds Intel ComboPhy driver, respective yaml schemas
Changes on v7:
As per System control driver maintainer's inputs remove
fwnode_to_regmap()definition and usedevice_node_get_regmap()
Changes on v6:
Rebase patches on the latest maintainer's branch
https://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git/?h=phy-for-5.7
Dilip Kota (3):
dt-bindings: phy: AddPHY_TYPE_XPCSdefinition
dt-bindings: phy: Add YAML schemas for Intel ComboPhy
phy: intel: Add driver support for ComboPhy.../devicetree/bindings/phy/intel,combo-phy.yaml | 101 ++++ drivers/phy/intel/Kconfig | 14 + drivers/phy/intel/Makefile | 1 + drivers/phy/intel/phy-intel-combo.c | 627 +++++++++++++++++++++ include/dt-bindings/phy/phy.h | 1 +
Quoted 5 lines
Quoted 1 line
Why have you sent this to me?
The main reason for this patch series is removing
fwnode_to_regmap()and usingdevice_node_get_regmap()compared to previous patch series. This has been done as per your review comments.
I see. Yes, dropping fwnode_to_regmap() was the right thing to do.
Quoted 2 lines
To keep you updated that changes are done as per your review comments, sent to you along with PHY maintainers.
Signature · 4 lines
Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog