* Re: [PATCH net-next 1/6] platform/x86: intel_pmc_core: Add IPC mailbox accessor function and add SoC register access [not found] ` <20230622041905.629430-2-yong.liang.choong@linux.intel.com> @ 2023-06-22 8:18 ` Hans de Goede 2023-06-23 5:52 ` Choong Yong Liang 2023-06-22 14:41 ` Simon Horman 1 sibling, 1 reply; 10+ messages in thread From: Hans de Goede @ 2023-06-22 8:18 UTC (permalink / raw) To: Choong Yong Liang, Rajneesh Bhardwaj, David E Box, Mark Gross, Jose Abreu, Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Marek Behún, Jean Delvare, Guenter Roeck, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime Coquelin, Richard Cochran, Philipp Zabel, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Wong Vee Khee, Jon Hunter, Jesse Brandeburg, Revanth Kumar Uppala, Shenwei Wang, Andrey Konovalov, Jochen Henneberg Cc: netdev, linux-kernel, linux-stm32, linux-arm-kernel, platform-driver-x86, linux-hwmon, bpf, Voon Wei Feng, Tee Min, Michael Sit Wei Hong, Lai Peter Jun Ann Hi, On 6/22/23 06:19, Choong Yong Liang wrote: > From: "David E. Box" <david.e.box@linux.intel.com> > > - Exports intel_pmc_core_ipc() for host access to the PMC IPC mailbox > - Add support to use IPC command allows host to access SoC registers > through PMC firmware that are otherwise inaccessible to the host due to > security policies. > > Signed-off-by: David E. Box <david.e.box@linux.intel.com> > Signed-off-by: Chao Qin <chao.qin@intel.com> > Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com> This seem to be 2 patches in 1: 1. Move core.h to include/linux/intel_pmc_core.h 2. The actual adding of IPC mailbox accessor function and add SoC register access I wonder if you really need to move the entire core.h ? IMHO it would be better to just add a new header with just the bits which you actually need to export the desired functionality. If you do believe that you really need to move core.h please split this into 2 separate patches and please place the header in a x86 specific place, e.g. : include/linux/platform_data/x86/ Also note that a somewhat big refactor, to add support for multiple PMCs for Meteor Lake is on its way to linux-next. Currently this is available in my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Please base a next version of this on this. There also is the question of how to merge this. Assuming this is ready for merging once 6.5-rc1 is out then I can merge this intel_pmc_core change into an immutable branch and send a pull-req to the net folks for this. Regards, Hans > --- > MAINTAINERS | 1 + > drivers/platform/x86/intel/pmc/adl.c | 2 +- > drivers/platform/x86/intel/pmc/cnp.c | 2 +- > drivers/platform/x86/intel/pmc/core.c | 63 ++++++++++++++++++- > drivers/platform/x86/intel/pmc/icl.c | 2 +- > drivers/platform/x86/intel/pmc/mtl.c | 2 +- > drivers/platform/x86/intel/pmc/spt.c | 2 +- > drivers/platform/x86/intel/pmc/tgl.c | 2 +- > .../core.h => include/linux/intel_pmc_core.h | 27 +++++++- > 9 files changed, 95 insertions(+), 8 deletions(-) > rename drivers/platform/x86/intel/pmc/core.h => include/linux/intel_pmc_core.h (95%) > > diff --git a/MAINTAINERS b/MAINTAINERS > index cb14589d14ab..bdb08a79a5f8 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -10581,6 +10581,7 @@ L: platform-driver-x86@vger.kernel.org > S: Maintained > F: Documentation/ABI/testing/sysfs-platform-intel-pmc > F: drivers/platform/x86/intel/pmc/ > +F: include/linux/intel_pmc_core* > > INTEL PMIC GPIO DRIVERS > M: Andy Shevchenko <andy@kernel.org> > diff --git a/drivers/platform/x86/intel/pmc/adl.c b/drivers/platform/x86/intel/pmc/adl.c > index 5cbd40979f2a..b6a376c536c0 100644 > --- a/drivers/platform/x86/intel/pmc/adl.c > +++ b/drivers/platform/x86/intel/pmc/adl.c > @@ -8,7 +8,7 @@ > * > */ > > -#include "core.h" > +#include <linux/intel_pmc_core.h> > > /* Alder Lake: PGD PFET Enable Ack Status Register(s) bitmap */ > const struct pmc_bit_map adl_pfear_map[] = { > diff --git a/drivers/platform/x86/intel/pmc/cnp.c b/drivers/platform/x86/intel/pmc/cnp.c > index 7fb38815c4eb..504034cc5ec3 100644 > --- a/drivers/platform/x86/intel/pmc/cnp.c > +++ b/drivers/platform/x86/intel/pmc/cnp.c > @@ -8,7 +8,7 @@ > * > */ > > -#include "core.h" > +#include <linux/intel_pmc_core.h> > > /* Cannon Lake: PGD PFET Enable Ack Status Register(s) bitmap */ > const struct pmc_bit_map cnp_pfear_map[] = { > diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c > index da6e7206d38b..0d60763c5144 100644 > --- a/drivers/platform/x86/intel/pmc/core.c > +++ b/drivers/platform/x86/intel/pmc/core.c > @@ -16,6 +16,7 @@ > #include <linux/delay.h> > #include <linux/dmi.h> > #include <linux/io.h> > +#include <linux/intel_pmc_core.h> > #include <linux/module.h> > #include <linux/pci.h> > #include <linux/slab.h> > @@ -26,7 +27,9 @@ > #include <asm/msr.h> > #include <asm/tsc.h> > > -#include "core.h" > +#define PMC_IPCS_PARAM_COUNT 7 > + > +static const struct x86_cpu_id *pmc_cpu_id; > > /* Maximum number of modes supported by platfoms that has low power mode capability */ > const char *pmc_lpm_modes[] = { > @@ -53,6 +56,63 @@ const struct pmc_bit_map msr_map[] = { > {} > }; > > +int intel_pmc_core_ipc(struct pmc_ipc_cmd *ipc_cmd, u32 *rbuf) > +{ > + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; > + union acpi_object params[PMC_IPCS_PARAM_COUNT] = { > + {.type = ACPI_TYPE_INTEGER,}, > + {.type = ACPI_TYPE_INTEGER,}, > + {.type = ACPI_TYPE_INTEGER,}, > + {.type = ACPI_TYPE_INTEGER,}, > + {.type = ACPI_TYPE_INTEGER,}, > + {.type = ACPI_TYPE_INTEGER,}, > + {.type = ACPI_TYPE_INTEGER,}, > + }; > + struct acpi_object_list arg_list = { PMC_IPCS_PARAM_COUNT, params }; > + union acpi_object *obj; > + int status; > + > + if (!pmc_cpu_id || !ipc_cmd || !rbuf) > + return -EINVAL; > + > + /* > + * 0: IPC Command > + * 1: IPC Sub Command > + * 2: Size > + * 3-6: Write Buffer for offset > + */ > + params[0].integer.value = ipc_cmd->cmd; > + params[1].integer.value = ipc_cmd->sub_cmd; > + params[2].integer.value = ipc_cmd->size; > + params[3].integer.value = ipc_cmd->wbuf[0]; > + params[4].integer.value = ipc_cmd->wbuf[1]; > + params[5].integer.value = ipc_cmd->wbuf[2]; > + params[6].integer.value = ipc_cmd->wbuf[3]; > + > + status = acpi_evaluate_object(NULL, "\\IPCS", &arg_list, &buffer); > + if (ACPI_FAILURE(status)) > + return -ENODEV; > + > + obj = buffer.pointer; > + /* Check if the number of elements in package is 5 */ > + if (obj && obj->type == ACPI_TYPE_PACKAGE && obj->package.count == 5) { > + const union acpi_object *objs = obj->package.elements; > + > + if ((u8)objs[0].integer.value != 0) > + return -EINVAL; > + > + rbuf[0] = objs[1].integer.value; > + rbuf[1] = objs[2].integer.value; > + rbuf[2] = objs[3].integer.value; > + rbuf[3] = objs[4].integer.value; > + } else { > + return -EINVAL; > + } > + > + return 0; > +} > +EXPORT_SYMBOL(intel_pmc_core_ipc); > + > static inline u32 pmc_core_reg_read(struct pmc_dev *pmcdev, int reg_offset) > { > return readl(pmcdev->regbase + reg_offset); > @@ -1130,6 +1190,7 @@ static int pmc_core_probe(struct platform_device *pdev) > mutex_init(&pmcdev->lock); > core_init(pmcdev); > > + pmc_cpu_id = cpu_id; > > if (lpit_read_residency_count_address(&slp_s0_addr)) { > pmcdev->base_addr = PMC_BASE_ADDR_DEFAULT; > diff --git a/drivers/platform/x86/intel/pmc/icl.c b/drivers/platform/x86/intel/pmc/icl.c > index 2f11b1a6daeb..f18048ff9382 100644 > --- a/drivers/platform/x86/intel/pmc/icl.c > +++ b/drivers/platform/x86/intel/pmc/icl.c > @@ -8,7 +8,7 @@ > * > */ > > -#include "core.h" > +#include <linux/intel_pmc_core.h> > > const struct pmc_bit_map icl_pfear_map[] = { > {"RES_65", BIT(0)}, > diff --git a/drivers/platform/x86/intel/pmc/mtl.c b/drivers/platform/x86/intel/pmc/mtl.c > index e8cc156412ce..7897f5fe9881 100644 > --- a/drivers/platform/x86/intel/pmc/mtl.c > +++ b/drivers/platform/x86/intel/pmc/mtl.c > @@ -9,7 +9,7 @@ > */ > > #include <linux/pci.h> > -#include "core.h" > +#include <linux/intel_pmc_core.h> > > const struct pmc_reg_map mtl_reg_map = { > .pfear_sts = ext_tgl_pfear_map, > diff --git a/drivers/platform/x86/intel/pmc/spt.c b/drivers/platform/x86/intel/pmc/spt.c > index e16982236778..95ce490cf5d6 100644 > --- a/drivers/platform/x86/intel/pmc/spt.c > +++ b/drivers/platform/x86/intel/pmc/spt.c > @@ -8,7 +8,7 @@ > * > */ > > -#include "core.h" > +#include <linux/intel_pmc_core.h> > > const struct pmc_bit_map spt_pll_map[] = { > {"MIPI PLL", SPT_PMC_BIT_MPHY_CMN_LANE0}, > diff --git a/drivers/platform/x86/intel/pmc/tgl.c b/drivers/platform/x86/intel/pmc/tgl.c > index c245ada849d0..a1719d809497 100644 > --- a/drivers/platform/x86/intel/pmc/tgl.c > +++ b/drivers/platform/x86/intel/pmc/tgl.c > @@ -8,7 +8,7 @@ > * > */ > > -#include "core.h" > +#include <linux/intel_pmc_core.h> > > #define ACPI_S0IX_DSM_UUID "57a6512e-3979-4e9d-9708-ff13b2508972" > #define ACPI_GET_LOW_MODE_REGISTERS 1 > diff --git a/drivers/platform/x86/intel/pmc/core.h b/include/linux/intel_pmc_core.h > similarity index 95% > rename from drivers/platform/x86/intel/pmc/core.h > rename to include/linux/intel_pmc_core.h > index 9ca9b9746719..82810e8b92a2 100644 > --- a/drivers/platform/x86/intel/pmc/core.h > +++ b/include/linux/intel_pmc_core.h > @@ -250,7 +250,16 @@ enum ppfear_regs { > #define MTL_LPM_STATUS_OFFSET 0x1700 > #define MTL_LPM_LIVE_STATUS_OFFSET 0x175C > > -extern const char *pmc_lpm_modes[]; > +#define IPC_SOC_REGISTER_ACCESS 0xAA > +#define IPC_SOC_SUB_CMD_READ 0x00 > +#define IPC_SOC_SUB_CMD_WRITE 0x01 > + > +struct pmc_ipc_cmd { > + u32 cmd; > + u32 sub_cmd; > + u32 size; > + u32 wbuf[4]; > +}; > > struct pmc_bit_map { > const char *name; > @@ -427,4 +436,20 @@ static const struct file_operations __name ## _fops = { \ > .release = single_release, \ > } > > +#if IS_ENABLED(CONFIG_INTEL_PMC_CORE) > +/** > + * intel_pmc_core_ipc() - PMC IPC Mailbox accessor > + * @ipc_cmd: struct pmc_ipc_cmd prepared with input to send > + * @rbuf: Allocated u32[4] array for returned IPC data > + * > + * Return: 0 on success. Non-zero on mailbox error > + */ > +int intel_pmc_core_ipc(struct pmc_ipc_cmd *ipc_cmd, u32 *rbuf); > +#else > +static inline int intel_pmc_core_ipc(struct pmc_ipc_cmd *ipc_cmd, u32 *rbuf) > +{ > + return -ENODEV; > +} > +#endif /* CONFIG_INTEL_PMC_CORE */ > + > #endif /* PMC_CORE_H */ _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 1/6] platform/x86: intel_pmc_core: Add IPC mailbox accessor function and add SoC register access 2023-06-22 8:18 ` [PATCH net-next 1/6] platform/x86: intel_pmc_core: Add IPC mailbox accessor function and add SoC register access Hans de Goede @ 2023-06-23 5:52 ` Choong Yong Liang 2023-06-23 12:44 ` Wong Vee Khee 0 siblings, 1 reply; 10+ messages in thread From: Choong Yong Liang @ 2023-06-23 5:52 UTC (permalink / raw) To: Hans de Goede, Rajneesh Bhardwaj, David E Box, Mark Gross, Jose Abreu, Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Marek Behún, Jean Delvare, Guenter Roeck, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime Coquelin, Richard Cochran, Philipp Zabel, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Wong Vee Khee, Jon Hunter, Jesse Brandeburg, Revanth Kumar Uppala, Shenwei Wang, Andrey Konovalov, Jochen Henneberg Cc: netdev, linux-kernel, linux-stm32, linux-arm-kernel, platform-driver-x86, linux-hwmon, bpf, Voon Wei Feng, Tee Min, Michael Sit Wei Hong, Lai Peter Jun Ann Hi Hans, I will discuss it with David but currently he is on vacation. It might take some time to get the final output. Thank you. On 22/6/2023 4:18 pm, Hans de Goede wrote: > Hi, > > On 6/22/23 06:19, Choong Yong Liang wrote: >> From: "David E. Box" <david.e.box@linux.intel.com> >> >> - Exports intel_pmc_core_ipc() for host access to the PMC IPC mailbox >> - Add support to use IPC command allows host to access SoC registers >> through PMC firmware that are otherwise inaccessible to the host due to >> security policies. >> >> Signed-off-by: David E. Box <david.e.box@linux.intel.com> >> Signed-off-by: Chao Qin <chao.qin@intel.com> >> Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com> > > This seem to be 2 patches in 1: > > 1. Move core.h to include/linux/intel_pmc_core.h > 2. The actual adding of IPC mailbox accessor function and add SoC register access > > I wonder if you really need to move the entire core.h ? > > IMHO it would be better to just add a new header with just the bits > which you actually need to export the desired functionality. > > If you do believe that you really need to move core.h please split > this into 2 separate patches and please place the header in a x86 > specific place, e.g. : include/linux/platform_data/x86/ > > > > Also note that a somewhat big refactor, to add support for > multiple PMCs for Meteor Lake is on its way to linux-next. > > Currently this is available in my review-hans branch: > > https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans > > Please base a next version of this on this. > > There also is the question of how to merge this. Assuming this is > ready for merging once 6.5-rc1 is out then I can merge this intel_pmc_core > change into an immutable branch and send a pull-req to the net folks > for this. > > Regards, > > Hans > > > > > > >> --- >> MAINTAINERS | 1 + >> drivers/platform/x86/intel/pmc/adl.c | 2 +- >> drivers/platform/x86/intel/pmc/cnp.c | 2 +- >> drivers/platform/x86/intel/pmc/core.c | 63 ++++++++++++++++++- >> drivers/platform/x86/intel/pmc/icl.c | 2 +- >> drivers/platform/x86/intel/pmc/mtl.c | 2 +- >> drivers/platform/x86/intel/pmc/spt.c | 2 +- >> drivers/platform/x86/intel/pmc/tgl.c | 2 +- >> .../core.h => include/linux/intel_pmc_core.h | 27 +++++++- >> 9 files changed, 95 insertions(+), 8 deletions(-) >> rename drivers/platform/x86/intel/pmc/core.h => include/linux/intel_pmc_core.h (95%) >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index cb14589d14ab..bdb08a79a5f8 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -10581,6 +10581,7 @@ L: platform-driver-x86@vger.kernel.org >> S: Maintained >> F: Documentation/ABI/testing/sysfs-platform-intel-pmc >> F: drivers/platform/x86/intel/pmc/ >> +F: include/linux/intel_pmc_core* >> >> INTEL PMIC GPIO DRIVERS >> M: Andy Shevchenko <andy@kernel.org> >> diff --git a/drivers/platform/x86/intel/pmc/adl.c b/drivers/platform/x86/intel/pmc/adl.c >> index 5cbd40979f2a..b6a376c536c0 100644 >> --- a/drivers/platform/x86/intel/pmc/adl.c >> +++ b/drivers/platform/x86/intel/pmc/adl.c >> @@ -8,7 +8,7 @@ >> * >> */ >> >> -#include "core.h" >> +#include <linux/intel_pmc_core.h> >> >> /* Alder Lake: PGD PFET Enable Ack Status Register(s) bitmap */ >> const struct pmc_bit_map adl_pfear_map[] = { >> diff --git a/drivers/platform/x86/intel/pmc/cnp.c b/drivers/platform/x86/intel/pmc/cnp.c >> index 7fb38815c4eb..504034cc5ec3 100644 >> --- a/drivers/platform/x86/intel/pmc/cnp.c >> +++ b/drivers/platform/x86/intel/pmc/cnp.c >> @@ -8,7 +8,7 @@ >> * >> */ >> >> -#include "core.h" >> +#include <linux/intel_pmc_core.h> >> >> /* Cannon Lake: PGD PFET Enable Ack Status Register(s) bitmap */ >> const struct pmc_bit_map cnp_pfear_map[] = { >> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c >> index da6e7206d38b..0d60763c5144 100644 >> --- a/drivers/platform/x86/intel/pmc/core.c >> +++ b/drivers/platform/x86/intel/pmc/core.c >> @@ -16,6 +16,7 @@ >> #include <linux/delay.h> >> #include <linux/dmi.h> >> #include <linux/io.h> >> +#include <linux/intel_pmc_core.h> >> #include <linux/module.h> >> #include <linux/pci.h> >> #include <linux/slab.h> >> @@ -26,7 +27,9 @@ >> #include <asm/msr.h> >> #include <asm/tsc.h> >> >> -#include "core.h" >> +#define PMC_IPCS_PARAM_COUNT 7 >> + >> +static const struct x86_cpu_id *pmc_cpu_id; >> >> /* Maximum number of modes supported by platfoms that has low power mode capability */ >> const char *pmc_lpm_modes[] = { >> @@ -53,6 +56,63 @@ const struct pmc_bit_map msr_map[] = { >> {} >> }; >> >> +int intel_pmc_core_ipc(struct pmc_ipc_cmd *ipc_cmd, u32 *rbuf) >> +{ >> + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; >> + union acpi_object params[PMC_IPCS_PARAM_COUNT] = { >> + {.type = ACPI_TYPE_INTEGER,}, >> + {.type = ACPI_TYPE_INTEGER,}, >> + {.type = ACPI_TYPE_INTEGER,}, >> + {.type = ACPI_TYPE_INTEGER,}, >> + {.type = ACPI_TYPE_INTEGER,}, >> + {.type = ACPI_TYPE_INTEGER,}, >> + {.type = ACPI_TYPE_INTEGER,}, >> + }; >> + struct acpi_object_list arg_list = { PMC_IPCS_PARAM_COUNT, params }; >> + union acpi_object *obj; >> + int status; >> + >> + if (!pmc_cpu_id || !ipc_cmd || !rbuf) >> + return -EINVAL; >> + >> + /* >> + * 0: IPC Command >> + * 1: IPC Sub Command >> + * 2: Size >> + * 3-6: Write Buffer for offset >> + */ >> + params[0].integer.value = ipc_cmd->cmd; >> + params[1].integer.value = ipc_cmd->sub_cmd; >> + params[2].integer.value = ipc_cmd->size; >> + params[3].integer.value = ipc_cmd->wbuf[0]; >> + params[4].integer.value = ipc_cmd->wbuf[1]; >> + params[5].integer.value = ipc_cmd->wbuf[2]; >> + params[6].integer.value = ipc_cmd->wbuf[3]; >> + >> + status = acpi_evaluate_object(NULL, "\\IPCS", &arg_list, &buffer); >> + if (ACPI_FAILURE(status)) >> + return -ENODEV; >> + >> + obj = buffer.pointer; >> + /* Check if the number of elements in package is 5 */ >> + if (obj && obj->type == ACPI_TYPE_PACKAGE && obj->package.count == 5) { >> + const union acpi_object *objs = obj->package.elements; >> + >> + if ((u8)objs[0].integer.value != 0) >> + return -EINVAL; >> + >> + rbuf[0] = objs[1].integer.value; >> + rbuf[1] = objs[2].integer.value; >> + rbuf[2] = objs[3].integer.value; >> + rbuf[3] = objs[4].integer.value; >> + } else { >> + return -EINVAL; >> + } >> + >> + return 0; >> +} >> +EXPORT_SYMBOL(intel_pmc_core_ipc); >> + >> static inline u32 pmc_core_reg_read(struct pmc_dev *pmcdev, int reg_offset) >> { >> return readl(pmcdev->regbase + reg_offset); >> @@ -1130,6 +1190,7 @@ static int pmc_core_probe(struct platform_device *pdev) >> mutex_init(&pmcdev->lock); >> core_init(pmcdev); >> >> + pmc_cpu_id = cpu_id; >> >> if (lpit_read_residency_count_address(&slp_s0_addr)) { >> pmcdev->base_addr = PMC_BASE_ADDR_DEFAULT; >> diff --git a/drivers/platform/x86/intel/pmc/icl.c b/drivers/platform/x86/intel/pmc/icl.c >> index 2f11b1a6daeb..f18048ff9382 100644 >> --- a/drivers/platform/x86/intel/pmc/icl.c >> +++ b/drivers/platform/x86/intel/pmc/icl.c >> @@ -8,7 +8,7 @@ >> * >> */ >> >> -#include "core.h" >> +#include <linux/intel_pmc_core.h> >> >> const struct pmc_bit_map icl_pfear_map[] = { >> {"RES_65", BIT(0)}, >> diff --git a/drivers/platform/x86/intel/pmc/mtl.c b/drivers/platform/x86/intel/pmc/mtl.c >> index e8cc156412ce..7897f5fe9881 100644 >> --- a/drivers/platform/x86/intel/pmc/mtl.c >> +++ b/drivers/platform/x86/intel/pmc/mtl.c >> @@ -9,7 +9,7 @@ >> */ >> >> #include <linux/pci.h> >> -#include "core.h" >> +#include <linux/intel_pmc_core.h> >> >> const struct pmc_reg_map mtl_reg_map = { >> .pfear_sts = ext_tgl_pfear_map, >> diff --git a/drivers/platform/x86/intel/pmc/spt.c b/drivers/platform/x86/intel/pmc/spt.c >> index e16982236778..95ce490cf5d6 100644 >> --- a/drivers/platform/x86/intel/pmc/spt.c >> +++ b/drivers/platform/x86/intel/pmc/spt.c >> @@ -8,7 +8,7 @@ >> * >> */ >> >> -#include "core.h" >> +#include <linux/intel_pmc_core.h> >> >> const struct pmc_bit_map spt_pll_map[] = { >> {"MIPI PLL", SPT_PMC_BIT_MPHY_CMN_LANE0}, >> diff --git a/drivers/platform/x86/intel/pmc/tgl.c b/drivers/platform/x86/intel/pmc/tgl.c >> index c245ada849d0..a1719d809497 100644 >> --- a/drivers/platform/x86/intel/pmc/tgl.c >> +++ b/drivers/platform/x86/intel/pmc/tgl.c >> @@ -8,7 +8,7 @@ >> * >> */ >> >> -#include "core.h" >> +#include <linux/intel_pmc_core.h> >> >> #define ACPI_S0IX_DSM_UUID "57a6512e-3979-4e9d-9708-ff13b2508972" >> #define ACPI_GET_LOW_MODE_REGISTERS 1 >> diff --git a/drivers/platform/x86/intel/pmc/core.h b/include/linux/intel_pmc_core.h >> similarity index 95% >> rename from drivers/platform/x86/intel/pmc/core.h >> rename to include/linux/intel_pmc_core.h >> index 9ca9b9746719..82810e8b92a2 100644 >> --- a/drivers/platform/x86/intel/pmc/core.h >> +++ b/include/linux/intel_pmc_core.h >> @@ -250,7 +250,16 @@ enum ppfear_regs { >> #define MTL_LPM_STATUS_OFFSET 0x1700 >> #define MTL_LPM_LIVE_STATUS_OFFSET 0x175C >> >> -extern const char *pmc_lpm_modes[]; >> +#define IPC_SOC_REGISTER_ACCESS 0xAA >> +#define IPC_SOC_SUB_CMD_READ 0x00 >> +#define IPC_SOC_SUB_CMD_WRITE 0x01 >> + >> +struct pmc_ipc_cmd { >> + u32 cmd; >> + u32 sub_cmd; >> + u32 size; >> + u32 wbuf[4]; >> +}; >> >> struct pmc_bit_map { >> const char *name; >> @@ -427,4 +436,20 @@ static const struct file_operations __name ## _fops = { \ >> .release = single_release, \ >> } >> >> +#if IS_ENABLED(CONFIG_INTEL_PMC_CORE) >> +/** >> + * intel_pmc_core_ipc() - PMC IPC Mailbox accessor >> + * @ipc_cmd: struct pmc_ipc_cmd prepared with input to send >> + * @rbuf: Allocated u32[4] array for returned IPC data >> + * >> + * Return: 0 on success. Non-zero on mailbox error >> + */ >> +int intel_pmc_core_ipc(struct pmc_ipc_cmd *ipc_cmd, u32 *rbuf); >> +#else >> +static inline int intel_pmc_core_ipc(struct pmc_ipc_cmd *ipc_cmd, u32 *rbuf) >> +{ >> + return -ENODEV; >> +} >> +#endif /* CONFIG_INTEL_PMC_CORE */ >> + >> #endif /* PMC_CORE_H */ > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 1/6] platform/x86: intel_pmc_core: Add IPC mailbox accessor function and add SoC register access 2023-06-23 5:52 ` Choong Yong Liang @ 2023-06-23 12:44 ` Wong Vee Khee 0 siblings, 0 replies; 10+ messages in thread From: Wong Vee Khee @ 2023-06-23 12:44 UTC (permalink / raw) To: Choong Yong Liang Cc: Hans de Goede, Rajneesh Bhardwaj, David E Box, Mark Gross, Jose Abreu, Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Marek Behún, Jean Delvare, Guenter Roeck, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime Coquelin, Richard Cochran, Philipp Zabel, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Wong Vee Khee, Jon Hunter, Jesse Brandeburg, Revanth Kumar Uppala, Shenwei Wang, Andrey Konovalov, Jochen Henneberg, netdev, linux-kernel, linux-stm32, linux-arm-kernel, platform-driver-x86, linux-hwmon, bpf, Voon Wei Feng, Tee Min, Michael Sit Wei Hong, Lai Peter Jun Ann On Fri, Jun 23, 2023 at 01:52:49PM +0800, Choong Yong Liang wrote: > Hi Hans, > > I will discuss it with David but currently he is on vacation. It might take > some time to get the final output. Thank you. > Please remember not to top post on ML. Regards, Vee Khee _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 1/6] platform/x86: intel_pmc_core: Add IPC mailbox accessor function and add SoC register access [not found] ` <20230622041905.629430-2-yong.liang.choong@linux.intel.com> 2023-06-22 8:18 ` [PATCH net-next 1/6] platform/x86: intel_pmc_core: Add IPC mailbox accessor function and add SoC register access Hans de Goede @ 2023-06-22 14:41 ` Simon Horman 2023-06-23 5:54 ` Choong Yong Liang 1 sibling, 1 reply; 10+ messages in thread From: Simon Horman @ 2023-06-22 14:41 UTC (permalink / raw) To: Choong Yong Liang Cc: Rajneesh Bhardwaj, David E Box, Hans de Goede, Mark Gross, Jose Abreu, Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Marek Behún, Jean Delvare, Guenter Roeck, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime Coquelin, Richard Cochran, Philipp Zabel, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Wong Vee Khee, Jon Hunter, Jesse Brandeburg, Revanth Kumar Uppala, Shenwei Wang, Andrey Konovalov, Jochen Henneberg, netdev, linux-kernel, linux-stm32, linux-arm-kernel, platform-driver-x86, linux-hwmon, bpf, Voon Wei Feng, Tan, Tee Min, Michael Sit Wei Hong, Lai Peter Jun Ann On Thu, Jun 22, 2023 at 12:19:00PM +0800, Choong Yong Liang wrote: > From: "David E. Box" <david.e.box@linux.intel.com> > > - Exports intel_pmc_core_ipc() for host access to the PMC IPC mailbox > - Add support to use IPC command allows host to access SoC registers > through PMC firmware that are otherwise inaccessible to the host due to > security policies. > > Signed-off-by: David E. Box <david.e.box@linux.intel.com> > Signed-off-by: Chao Qin <chao.qin@intel.com> > Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com> ... > diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c > index da6e7206d38b..0d60763c5144 100644 > --- a/drivers/platform/x86/intel/pmc/core.c > +++ b/drivers/platform/x86/intel/pmc/core.c > @@ -16,6 +16,7 @@ > #include <linux/delay.h> > #include <linux/dmi.h> > #include <linux/io.h> > +#include <linux/intel_pmc_core.h> > #include <linux/module.h> > #include <linux/pci.h> > #include <linux/slab.h> > @@ -26,7 +27,9 @@ > #include <asm/msr.h> > #include <asm/tsc.h> > > -#include "core.h" > +#define PMC_IPCS_PARAM_COUNT 7 > + > +static const struct x86_cpu_id *pmc_cpu_id; > > /* Maximum number of modes supported by platfoms that has low power mode capability */ > const char *pmc_lpm_modes[] = { Hi Choong Yong Liang, It looks like pmc_lpm_mode is used in this file and, as of this patch, has no declaration. Should it be static? ... > diff --git a/drivers/platform/x86/intel/pmc/core.h b/include/linux/intel_pmc_core.h > similarity index 95% > rename from drivers/platform/x86/intel/pmc/core.h > rename to include/linux/intel_pmc_core.h > index 9ca9b9746719..82810e8b92a2 100644 > --- a/drivers/platform/x86/intel/pmc/core.h > +++ b/include/linux/intel_pmc_core.h > @@ -250,7 +250,16 @@ enum ppfear_regs { > #define MTL_LPM_STATUS_OFFSET 0x1700 > #define MTL_LPM_LIVE_STATUS_OFFSET 0x175C > > -extern const char *pmc_lpm_modes[]; > +#define IPC_SOC_REGISTER_ACCESS 0xAA > +#define IPC_SOC_SUB_CMD_READ 0x00 > +#define IPC_SOC_SUB_CMD_WRITE 0x01 > + > +struct pmc_ipc_cmd { > + u32 cmd; > + u32 sub_cmd; > + u32 size; > + u32 wbuf[4]; > +}; > > struct pmc_bit_map { > const char *name; ... _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 1/6] platform/x86: intel_pmc_core: Add IPC mailbox accessor function and add SoC register access 2023-06-22 14:41 ` Simon Horman @ 2023-06-23 5:54 ` Choong Yong Liang 0 siblings, 0 replies; 10+ messages in thread From: Choong Yong Liang @ 2023-06-23 5:54 UTC (permalink / raw) To: Simon Horman Cc: Rajneesh Bhardwaj, David E Box, Hans de Goede, Mark Gross, Jose Abreu, Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Marek Behún, Jean Delvare, Guenter Roeck, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime Coquelin, Richard Cochran, Philipp Zabel, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Wong Vee Khee, Jon Hunter, Jesse Brandeburg, Revanth Kumar Uppala, Shenwei Wang, Andrey Konovalov, Jochen Henneberg, netdev, linux-kernel, linux-stm32, linux-arm-kernel, platform-driver-x86, linux-hwmon, bpf, Voon Wei Feng, Tan, Tee Min, Michael Sit Wei Hong, Lai Peter Jun Ann Hi Simon, Yes, you are right. I will add static in v2. Thank you. On 22/6/2023 10:41 pm, Simon Horman wrote: > On Thu, Jun 22, 2023 at 12:19:00PM +0800, Choong Yong Liang wrote: >> From: "David E. Box" <david.e.box@linux.intel.com> >> >> - Exports intel_pmc_core_ipc() for host access to the PMC IPC mailbox >> - Add support to use IPC command allows host to access SoC registers >> through PMC firmware that are otherwise inaccessible to the host due to >> security policies. >> >> Signed-off-by: David E. Box <david.e.box@linux.intel.com> >> Signed-off-by: Chao Qin <chao.qin@intel.com> >> Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com> > > ... > >> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c >> index da6e7206d38b..0d60763c5144 100644 >> --- a/drivers/platform/x86/intel/pmc/core.c >> +++ b/drivers/platform/x86/intel/pmc/core.c >> @@ -16,6 +16,7 @@ >> #include <linux/delay.h> >> #include <linux/dmi.h> >> #include <linux/io.h> >> +#include <linux/intel_pmc_core.h> >> #include <linux/module.h> >> #include <linux/pci.h> >> #include <linux/slab.h> >> @@ -26,7 +27,9 @@ >> #include <asm/msr.h> >> #include <asm/tsc.h> >> >> -#include "core.h" >> +#define PMC_IPCS_PARAM_COUNT 7 >> + >> +static const struct x86_cpu_id *pmc_cpu_id; >> >> /* Maximum number of modes supported by platfoms that has low power mode capability */ >> const char *pmc_lpm_modes[] = { > > Hi Choong Yong Liang, > > It looks like pmc_lpm_mode is used in this file and, as of this patch, > has no declaration. Should it be static? > > ... > >> diff --git a/drivers/platform/x86/intel/pmc/core.h b/include/linux/intel_pmc_core.h >> similarity index 95% >> rename from drivers/platform/x86/intel/pmc/core.h >> rename to include/linux/intel_pmc_core.h >> index 9ca9b9746719..82810e8b92a2 100644 >> --- a/drivers/platform/x86/intel/pmc/core.h >> +++ b/include/linux/intel_pmc_core.h >> @@ -250,7 +250,16 @@ enum ppfear_regs { >> #define MTL_LPM_STATUS_OFFSET 0x1700 >> #define MTL_LPM_LIVE_STATUS_OFFSET 0x175C >> >> -extern const char *pmc_lpm_modes[]; >> +#define IPC_SOC_REGISTER_ACCESS 0xAA >> +#define IPC_SOC_SUB_CMD_READ 0x00 >> +#define IPC_SOC_SUB_CMD_WRITE 0x01 >> + >> +struct pmc_ipc_cmd { >> + u32 cmd; >> + u32 sub_cmd; >> + u32 size; >> + u32 wbuf[4]; >> +}; >> >> struct pmc_bit_map { >> const char *name; > > ... _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20230622041905.629430-4-yong.liang.choong@linux.intel.com>]
* Re: [PATCH net-next 3/6] net: phy: update in-band AN mode when changing interface by PHY driver [not found] ` <20230622041905.629430-4-yong.liang.choong@linux.intel.com> @ 2023-06-22 14:43 ` Simon Horman 2023-06-22 15:06 ` Russell King (Oracle) 2023-06-23 5:57 ` Choong Yong Liang 0 siblings, 2 replies; 10+ messages in thread From: Simon Horman @ 2023-06-22 14:43 UTC (permalink / raw) To: Choong Yong Liang Cc: Rajneesh Bhardwaj, David E Box, Hans de Goede, Mark Gross, Jose Abreu, Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Marek Behún, Jean Delvare, Guenter Roeck, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime Coquelin, Richard Cochran, Philipp Zabel, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Wong Vee Khee, Jon Hunter, Jesse Brandeburg, Revanth Kumar Uppala, Shenwei Wang, Andrey Konovalov, Jochen Henneberg, netdev, linux-kernel, linux-stm32, linux-arm-kernel, platform-driver-x86, linux-hwmon, bpf, Voon Wei Feng, Tan, Tee Min, Michael Sit Wei Hong, Lai Peter Jun Ann On Thu, Jun 22, 2023 at 12:19:02PM +0800, Choong Yong Liang wrote: > From: "Tan, Tee Min" <tee.min.tan@linux.intel.com> > > Add cur_link_an_mode into phy_device struct for PHY drivers to > communicate the in-band AN mode setting with phylink framework. > > As there is a mechanism in PHY drivers to switch the PHY interface > between SGMII and 2500BaseX according to link speed. In this case, > the in-band AN mode should be switching based on the PHY interface > as well, if the PHY interface has been changed/updated by PHY driver. > > For e.g., disable in-band AN in 2500BaseX mode, or enable in-band AN > back for SGMII mode (10/100/1000Mbps). > > Signed-off-by: Tan, Tee Min <tee.min.tan@linux.intel.com> > Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com> ... > diff --git a/include/linux/phy.h b/include/linux/phy.h > index 11c1e91563d4..c685b526e307 100644 > --- a/include/linux/phy.h > +++ b/include/linux/phy.h > @@ -756,6 +756,8 @@ struct phy_device { > /* MACsec management functions */ > const struct macsec_ops *macsec_ops; > #endif > + /* For communicate the AN mode setting with phylink framework. */ > + u8 cur_link_an_mode; > }; Hi Choong Yong Liang, Please consider adding cur_link_an_mode to the kernel doc for struct phy_device - which is above the definition of struct phy_device. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 3/6] net: phy: update in-band AN mode when changing interface by PHY driver 2023-06-22 14:43 ` [PATCH net-next 3/6] net: phy: update in-band AN mode when changing interface by PHY driver Simon Horman @ 2023-06-22 15:06 ` Russell King (Oracle) 2023-06-23 5:35 ` Simon Horman 2023-06-23 6:02 ` Choong Yong Liang 2023-06-23 5:57 ` Choong Yong Liang 1 sibling, 2 replies; 10+ messages in thread From: Russell King (Oracle) @ 2023-06-22 15:06 UTC (permalink / raw) To: Simon Horman Cc: Choong Yong Liang, Rajneesh Bhardwaj, David E Box, Hans de Goede, Mark Gross, Jose Abreu, Andrew Lunn, Heiner Kallweit, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Marek Behún, Jean Delvare, Guenter Roeck, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime Coquelin, Richard Cochran, Philipp Zabel, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Wong Vee Khee, Jon Hunter, Jesse Brandeburg, Revanth Kumar Uppala, Shenwei Wang, Andrey Konovalov, Jochen Henneberg, netdev, linux-kernel, linux-stm32, linux-arm-kernel, platform-driver-x86, linux-hwmon, bpf, Voon Wei Feng, Tan, Tee Min, Michael Sit Wei Hong, Lai Peter Jun Ann On Thu, Jun 22, 2023 at 04:43:51PM +0200, Simon Horman wrote: > On Thu, Jun 22, 2023 at 12:19:02PM +0800, Choong Yong Liang wrote: > > From: "Tan, Tee Min" <tee.min.tan@linux.intel.com> > > > > Add cur_link_an_mode into phy_device struct for PHY drivers to > > communicate the in-band AN mode setting with phylink framework. > > > > As there is a mechanism in PHY drivers to switch the PHY interface > > between SGMII and 2500BaseX according to link speed. In this case, > > the in-band AN mode should be switching based on the PHY interface > > as well, if the PHY interface has been changed/updated by PHY driver. > > > > For e.g., disable in-band AN in 2500BaseX mode, or enable in-band AN > > back for SGMII mode (10/100/1000Mbps). > > > > Signed-off-by: Tan, Tee Min <tee.min.tan@linux.intel.com> > > Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com> > > ... > > > diff --git a/include/linux/phy.h b/include/linux/phy.h > > index 11c1e91563d4..c685b526e307 100644 > > --- a/include/linux/phy.h > > +++ b/include/linux/phy.h > > @@ -756,6 +756,8 @@ struct phy_device { > > /* MACsec management functions */ > > const struct macsec_ops *macsec_ops; > > #endif > > + /* For communicate the AN mode setting with phylink framework. */ > > + u8 cur_link_an_mode; > > }; > > Hi Choong Yong Liang, > > Please consider adding cur_link_an_mode to the kernel doc > for struct phy_device - which is above the definition of struct phy_device. This looks like it's grabbing something from phylink and stuffing it into phylib. However, I have no idea, because I don't seem to have received the original patches. I'm guessing the reason is: 2023-06-22 05:21:24 1qCBoy-0003ji-G9 H=mga03.intel.com [134.134.136.65]:57703 I=[78.32.30.218]:25 X=TLS1.2:ECDHE_SECP521R1__RSA_SHA512__AES_256_GCM:256 F=<yong.liang.choong@linux.intel.com> rejected after DATA: unqualified address not permitted: failing address in "Cc:" header is: Tan Which I suspect came from: Tan, Tee Min <tee.min.tan@linux.intel.com> and someone doesn't realise that a "," in the display-name part of an address *must* be quoted, otherwise "," is taken to be a separator in the address list. Consequently, it has now become: Tan@web.codeaurora.org, Tee Min <tee.min.tan@linux.intel.com>, It should have been: "Tan, Tee Min" <tee.min.tan@linux.intel.com> with the double-quotes. Please do not review this series further, but instead, please can the author repost it forthwith with correct conformant headers so that a proper review can be undertaken by all? Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 3/6] net: phy: update in-band AN mode when changing interface by PHY driver 2023-06-22 15:06 ` Russell King (Oracle) @ 2023-06-23 5:35 ` Simon Horman 2023-06-23 6:02 ` Choong Yong Liang 1 sibling, 0 replies; 10+ messages in thread From: Simon Horman @ 2023-06-23 5:35 UTC (permalink / raw) To: Russell King (Oracle) Cc: Choong Yong Liang, Rajneesh Bhardwaj, David E Box, Hans de Goede, Mark Gross, Jose Abreu, Andrew Lunn, Heiner Kallweit, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Marek Behún, Jean Delvare, Guenter Roeck, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime Coquelin, Richard Cochran, Philipp Zabel, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Wong Vee Khee, Jon Hunter, Jesse Brandeburg, Revanth Kumar Uppala, Shenwei Wang, Andrey Konovalov, Jochen Henneberg, netdev, linux-kernel, linux-stm32, linux-arm-kernel, platform-driver-x86, linux-hwmon, bpf, Voon Wei Feng, Tan, Tee Min, Michael Sit Wei Hong, Lai Peter Jun Ann On Thu, Jun 22, 2023 at 04:06:31PM +0100, Russell King (Oracle) wrote: > On Thu, Jun 22, 2023 at 04:43:51PM +0200, Simon Horman wrote: > > On Thu, Jun 22, 2023 at 12:19:02PM +0800, Choong Yong Liang wrote: > > > From: "Tan, Tee Min" <tee.min.tan@linux.intel.com> > > > > > > Add cur_link_an_mode into phy_device struct for PHY drivers to > > > communicate the in-band AN mode setting with phylink framework. > > > > > > As there is a mechanism in PHY drivers to switch the PHY interface > > > between SGMII and 2500BaseX according to link speed. In this case, > > > the in-band AN mode should be switching based on the PHY interface > > > as well, if the PHY interface has been changed/updated by PHY driver. > > > > > > For e.g., disable in-band AN in 2500BaseX mode, or enable in-band AN > > > back for SGMII mode (10/100/1000Mbps). > > > > > > Signed-off-by: Tan, Tee Min <tee.min.tan@linux.intel.com> > > > Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com> > > > > ... > > > > > diff --git a/include/linux/phy.h b/include/linux/phy.h > > > index 11c1e91563d4..c685b526e307 100644 > > > --- a/include/linux/phy.h > > > +++ b/include/linux/phy.h > > > @@ -756,6 +756,8 @@ struct phy_device { > > > /* MACsec management functions */ > > > const struct macsec_ops *macsec_ops; > > > #endif > > > + /* For communicate the AN mode setting with phylink framework. */ > > > + u8 cur_link_an_mode; > > > }; > > > > Hi Choong Yong Liang, > > > > Please consider adding cur_link_an_mode to the kernel doc > > for struct phy_device - which is above the definition of struct phy_device. > > This looks like it's grabbing something from phylink and stuffing it > into phylib. However, I have no idea, because I don't seem to have > received the original patches. I'm guessing the reason is: > > 2023-06-22 05:21:24 1qCBoy-0003ji-G9 H=mga03.intel.com > [134.134.136.65]:57703 I=[78.32.30.218]:25 > X=TLS1.2:ECDHE_SECP521R1__RSA_SHA512__AES_256_GCM:256 > F=<yong.liang.choong@linux.intel.com> rejected after DATA: unqualified > address not permitted: failing address in "Cc:" header is: Tan > > Which I suspect came from: > > Tan, Tee Min <tee.min.tan@linux.intel.com> > > and someone doesn't realise that a "," in the display-name part of > an address *must* be quoted, otherwise "," is taken to be a separator > in the address list. > > Consequently, it has now become: > > Tan@web.codeaurora.org, Tee Min <tee.min.tan@linux.intel.com>, > > It should have been: > > "Tan, Tee Min" <tee.min.tan@linux.intel.com> > > with the double-quotes. > > Please do not review this series further, but instead, please can the > author repost it forthwith with correct conformant headers so that a > proper review can be undertaken by all? Hi Russell, Sorry for not noticing this myself. I agree that we should wait for a properly formed post as you suggest. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 3/6] net: phy: update in-band AN mode when changing interface by PHY driver 2023-06-22 15:06 ` Russell King (Oracle) 2023-06-23 5:35 ` Simon Horman @ 2023-06-23 6:02 ` Choong Yong Liang 1 sibling, 0 replies; 10+ messages in thread From: Choong Yong Liang @ 2023-06-23 6:02 UTC (permalink / raw) To: Russell King (Oracle), Simon Horman Cc: Rajneesh Bhardwaj, David E Box, Hans de Goede, Mark Gross, Jose Abreu, Andrew Lunn, Heiner Kallweit, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Marek Behún, Jean Delvare, Guenter Roeck, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime Coquelin, Richard Cochran, Philipp Zabel, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Wong Vee Khee, Jon Hunter, Jesse Brandeburg, Revanth Kumar Uppala, Shenwei Wang, Andrey Konovalov, Jochen Henneberg, netdev, linux-kernel, linux-stm32, linux-arm-kernel, platform-driver-x86, linux-hwmon, bpf, Voon Wei Feng, Tan, Tee Min, Michael Sit Wei Hong, Lai Peter Jun Ann Hi Russell, Thank you for pointing that out. I will fix it and send out version 2. On 22/6/2023 11:06 pm, Russell King (Oracle) wrote: > On Thu, Jun 22, 2023 at 04:43:51PM +0200, Simon Horman wrote: >> On Thu, Jun 22, 2023 at 12:19:02PM +0800, Choong Yong Liang wrote: >>> From: "Tan, Tee Min" <tee.min.tan@linux.intel.com> >>> >>> Add cur_link_an_mode into phy_device struct for PHY drivers to >>> communicate the in-band AN mode setting with phylink framework. >>> >>> As there is a mechanism in PHY drivers to switch the PHY interface >>> between SGMII and 2500BaseX according to link speed. In this case, >>> the in-band AN mode should be switching based on the PHY interface >>> as well, if the PHY interface has been changed/updated by PHY driver. >>> >>> For e.g., disable in-band AN in 2500BaseX mode, or enable in-band AN >>> back for SGMII mode (10/100/1000Mbps). >>> >>> Signed-off-by: Tan, Tee Min <tee.min.tan@linux.intel.com> >>> Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com> >> >> ... >> >>> diff --git a/include/linux/phy.h b/include/linux/phy.h >>> index 11c1e91563d4..c685b526e307 100644 >>> --- a/include/linux/phy.h >>> +++ b/include/linux/phy.h >>> @@ -756,6 +756,8 @@ struct phy_device { >>> /* MACsec management functions */ >>> const struct macsec_ops *macsec_ops; >>> #endif >>> + /* For communicate the AN mode setting with phylink framework. */ >>> + u8 cur_link_an_mode; >>> }; >> >> Hi Choong Yong Liang, >> >> Please consider adding cur_link_an_mode to the kernel doc >> for struct phy_device - which is above the definition of struct phy_device. > > This looks like it's grabbing something from phylink and stuffing it > into phylib. However, I have no idea, because I don't seem to have > received the original patches. I'm guessing the reason is: > > 2023-06-22 05:21:24 1qCBoy-0003ji-G9 H=mga03.intel.com > [134.134.136.65]:57703 I=[78.32.30.218]:25 > X=TLS1.2:ECDHE_SECP521R1__RSA_SHA512__AES_256_GCM:256 > F=<yong.liang.choong@linux.intel.com> rejected after DATA: unqualified > address not permitted: failing address in "Cc:" header is: Tan > > Which I suspect came from: > > Tan, Tee Min <tee.min.tan@linux.intel.com> > > and someone doesn't realise that a "," in the display-name part of > an address *must* be quoted, otherwise "," is taken to be a separator > in the address list. > > Consequently, it has now become: > > Tan@web.codeaurora.org, Tee Min <tee.min.tan@linux.intel.com>, > > It should have been: > > "Tan, Tee Min" <tee.min.tan@linux.intel.com> > > with the double-quotes. > > Please do not review this series further, but instead, please can the > author repost it forthwith with correct conformant headers so that a > proper review can be undertaken by all? > > Thanks. > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next 3/6] net: phy: update in-band AN mode when changing interface by PHY driver 2023-06-22 14:43 ` [PATCH net-next 3/6] net: phy: update in-band AN mode when changing interface by PHY driver Simon Horman 2023-06-22 15:06 ` Russell King (Oracle) @ 2023-06-23 5:57 ` Choong Yong Liang 1 sibling, 0 replies; 10+ messages in thread From: Choong Yong Liang @ 2023-06-23 5:57 UTC (permalink / raw) To: Simon Horman Cc: Rajneesh Bhardwaj, David E Box, Hans de Goede, Mark Gross, Jose Abreu, Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Marek Behún, Jean Delvare, Guenter Roeck, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, Maxime Coquelin, Richard Cochran, Philipp Zabel, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, Wong Vee Khee, Jon Hunter, Jesse Brandeburg, Revanth Kumar Uppala, Shenwei Wang, Andrey Konovalov, Jochen Henneberg, netdev, linux-kernel, linux-stm32, linux-arm-kernel, platform-driver-x86, linux-hwmon, bpf, Voon Wei Feng, Tee Min, Michael Sit Wei Hong, Lai Peter Jun Ann Hi Simon, I will update it in v2. Thank you. On 22/6/2023 10:43 pm, Simon Horman wrote: > On Thu, Jun 22, 2023 at 12:19:02PM +0800, Choong Yong Liang wrote: >> From: "Tan, Tee Min" <tee.min.tan@linux.intel.com> >> >> Add cur_link_an_mode into phy_device struct for PHY drivers to >> communicate the in-band AN mode setting with phylink framework. >> >> As there is a mechanism in PHY drivers to switch the PHY interface >> between SGMII and 2500BaseX according to link speed. In this case, >> the in-band AN mode should be switching based on the PHY interface >> as well, if the PHY interface has been changed/updated by PHY driver. >> >> For e.g., disable in-band AN in 2500BaseX mode, or enable in-band AN >> back for SGMII mode (10/100/1000Mbps). >> >> Signed-off-by: Tan, Tee Min <tee.min.tan@linux.intel.com> >> Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com> > > ... > >> diff --git a/include/linux/phy.h b/include/linux/phy.h >> index 11c1e91563d4..c685b526e307 100644 >> --- a/include/linux/phy.h >> +++ b/include/linux/phy.h >> @@ -756,6 +756,8 @@ struct phy_device { >> /* MACsec management functions */ >> const struct macsec_ops *macsec_ops; >> #endif >> + /* For communicate the AN mode setting with phylink framework. */ >> + u8 cur_link_an_mode; >> }; > > Hi Choong Yong Liang, > > Please consider adding cur_link_an_mode to the kernel doc > for struct phy_device - which is above the definition of struct phy_device. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-06-23 12:45 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20230622041905.629430-1-yong.liang.choong@linux.intel.com>
[not found] ` <20230622041905.629430-2-yong.liang.choong@linux.intel.com>
2023-06-22 8:18 ` [PATCH net-next 1/6] platform/x86: intel_pmc_core: Add IPC mailbox accessor function and add SoC register access Hans de Goede
2023-06-23 5:52 ` Choong Yong Liang
2023-06-23 12:44 ` Wong Vee Khee
2023-06-22 14:41 ` Simon Horman
2023-06-23 5:54 ` Choong Yong Liang
[not found] ` <20230622041905.629430-4-yong.liang.choong@linux.intel.com>
2023-06-22 14:43 ` [PATCH net-next 3/6] net: phy: update in-band AN mode when changing interface by PHY driver Simon Horman
2023-06-22 15:06 ` Russell King (Oracle)
2023-06-23 5:35 ` Simon Horman
2023-06-23 6:02 ` Choong Yong Liang
2023-06-23 5:57 ` Choong Yong Liang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).