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: <CAMuE1bE-RCLSWX-N5TmGPqXBEuHJAwLNJSOk=w-2XUDhrwYjAQ@mail.gmail.com> Date: Sun, 24 Dec 2023 13:56:09 +0200 From: Sagi Maimon <maimon.sagi@...il.com> To: Vadim Fedorenko <vadim.fedorenko@...ux.dev> Cc: richardcochran@...il.com, jonathan.lemon@...il.com, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH v1] ptp: ocp: add Adva timecard support On Thu, Dec 21, 2023 at 6:56 PM Vadim Fedorenko <vadim.fedorenko@...ux.dev> wrote: Vadim Thanks for your comments. > > On 21/12/2023 15:37, Sagi Maimon wrote: > > Adding support for the Adva timecard. > > The card uses different drivers to provide access to the > > firmware SPI flash (Altera based). > > Other parts of the code are the same and could be reused. > > > > Signed-off-by: Sagi Maimon <maimon.sagi@...il.com> > > --- > > drivers/ptp/ptp_ocp.c | 257 ++++++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 247 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c > > index 4021d3d325f9..73e91b8a2887 100644 > > --- a/drivers/ptp/ptp_ocp.c > > +++ b/drivers/ptp/ptp_ocp.c > > @@ -34,6 +34,9 @@ > > #define PCI_VENDOR_ID_OROLIA 0x1ad7 > > #define PCI_DEVICE_ID_OROLIA_ARTCARD 0xa000 > > > > +#define PCI_VENDOR_ID_ADVA 0xad5a > > +#define PCI_DEVICE_ID_ADVA_TIMECARD 0x0400 > > + > > static struct class timecard_class = { > > .name = "timecard", > > }; > > @@ -63,6 +66,13 @@ struct ocp_reg { > > u32 status_drift; > > }; > > > > +struct servo_val { > > + u32 servo_offset_p_val; > > + u32 servo_offset_i_val; > > + u32 servo_drift_p_val; > > + u32 servo_drift_i_val; > > +}; > > + > > I don't really like naming here. Let's go with ptp_ocp prefix first. > Then it's more like configuration rather than actual values, so I would > say ptp_ocp_servo_conf is better here. And let's remove "_val" - no real > need for this suffix. > Will be fixed on patch V2 > > #define OCP_CTRL_ENABLE BIT(0) > > #define OCP_CTRL_ADJUST_TIME BIT(1) > > #define OCP_CTRL_ADJUST_OFFSET BIT(2) > > @@ -401,6 +411,12 @@ static const struct ocp_attr_group fb_timecard_groups[]; > > > > static const struct ocp_attr_group art_timecard_groups[]; > > > > +static int ptp_ocp_adva_board_init(struct ptp_ocp *bp, struct ocp_resource *r); > > + > > +static const struct ocp_attr_group adva_timecard_groups[]; > > + > > +static const struct ocp_sma_op ocp_adva_sma_op; > > + > > struct ptp_ocp_eeprom_map { > > u16 off; > > u16 len; > > @@ -835,10 +851,122 @@ static struct ocp_resource ocp_art_resource[] = { > > { } > > }; > > > > +static struct ocp_resource ocp_adva_resource[] = { > > + { > > + OCP_MEM_RESOURCE(reg), > > + .offset = 0x01000000, .size = 0x10000, > > + }, > > + { > > + OCP_EXT_RESOURCE(ts0), > > + .offset = 0x01010000, .size = 0x10000, .irq_vec = 1, > > + .extra = &(struct ptp_ocp_ext_info) { > > + .index = 0, > > + .irq_fcn = ptp_ocp_ts_irq, > > + .enable = ptp_ocp_ts_enable, > > + }, > > + }, > > + { > > + OCP_EXT_RESOURCE(ts1), > > + .offset = 0x01020000, .size = 0x10000, .irq_vec = 2, > > + .extra = &(struct ptp_ocp_ext_info) { > > + .index = 1, > > + .irq_fcn = ptp_ocp_ts_irq, > > + .enable = ptp_ocp_ts_enable, > > + }, > > + }, > > + { > > + OCP_EXT_RESOURCE(ts2), > > + .offset = 0x01060000, .size = 0x10000, .irq_vec = 6, > > + .extra = &(struct ptp_ocp_ext_info) { > > + .index = 2, > > + .irq_fcn = ptp_ocp_ts_irq, > > + .enable = ptp_ocp_ts_enable, > > + }, > > + }, > > + /* Timestamp for PHC and/or PPS generator */ > > + { > > + OCP_EXT_RESOURCE(pps), > > + .offset = 0x010C0000, .size = 0x10000, .irq_vec = 0, > > + .extra = &(struct ptp_ocp_ext_info) { > > + .index = 5, > > + .irq_fcn = ptp_ocp_ts_irq, > > + .enable = ptp_ocp_ts_enable, > > + }, > > + }, > > + { > > + OCP_EXT_RESOURCE(signal_out[0]), > > + .offset = 0x010D0000, .size = 0x10000, .irq_vec = 11, > > + .extra = &(struct ptp_ocp_ext_info) { > > + .index = 1, > > + .irq_fcn = ptp_ocp_signal_irq, > > + .enable = ptp_ocp_signal_enable, > > + }, > > + }, > > + { > > + OCP_MEM_RESOURCE(pps_to_ext), > > + .offset = 0x01030000, .size = 0x10000, > > + }, > > + { > > + OCP_MEM_RESOURCE(pps_to_clk), > > + .offset = 0x01040000, .size = 0x10000, > > + }, > > + { > > + OCP_MEM_RESOURCE(tod), > > + .offset = 0x01050000, .size = 0x10000, > > + }, > > + { > > + OCP_MEM_RESOURCE(image), > > + .offset = 0x00020000, .size = 0x1000, > > + }, > > + { > > + OCP_MEM_RESOURCE(pps_select), > > + .offset = 0x00130000, .size = 0x1000, > > + }, > > + { > > + OCP_MEM_RESOURCE(sma_map1), > > + .offset = 0x00140000, .size = 0x1000, > > + }, > > + { > > + OCP_MEM_RESOURCE(sma_map2), > > + .offset = 0x00220000, .size = 0x1000, > > + }, > > + { > > + OCP_SERIAL_RESOURCE(gnss_port), > > + .offset = 0x00160000 + 0x1000, .irq_vec = 3, > > + .extra = &(struct ptp_ocp_serial_port) { > > + .baud = 9600, > > + }, > > + }, > > + { > > + OCP_MEM_RESOURCE(freq_in[0]), > > + .offset = 0x01200000, .size = 0x10000, > > + }, > > + { > > + OCP_SPI_RESOURCE(spi_flash), > > + .offset = 0x00310400, .size = 0x10000, .irq_vec = 9, > > + .extra = &(struct ptp_ocp_flash_info) { > > + .name = "spi_altera", .pci_offset = 0, > > + .data_size = sizeof(struct altera_spi_platform_data), > > + .data = &(struct altera_spi_platform_data) { > > + .num_chipselect = 1, > > + .num_devices = 1, > > + .devices = &(struct spi_board_info) { > > + .modalias = "spi-nor", > > + }, > > + }, > > + }, > > + }, > > + { > > + .setup = ptp_ocp_adva_board_init, > > + }, > > + { } > > +}; > > + > > static const struct pci_device_id ptp_ocp_pcidev_id[] = { > > { PCI_DEVICE_DATA(FACEBOOK, TIMECARD, &ocp_fb_resource) }, > > { PCI_DEVICE_DATA(CELESTICA, TIMECARD, &ocp_fb_resource) }, > > { PCI_DEVICE_DATA(OROLIA, ARTCARD, &ocp_art_resource) }, > > + { PCI_DEVICE_DATA(ADVA, TIMECARD, &ocp_adva_resource) }, > > { } > > }; > > MODULE_DEVICE_TABLE(pci, ptp_ocp_pcidev_id); > > @@ -917,6 +1045,27 @@ static const struct ocp_selector ptp_ocp_art_sma_out[] = { > > { } > > }; > > > > +static const struct ocp_selector ptp_ocp_adva_sma_in[] = { > > + { .name = "10Mhz", .value = 0x0000, .frequency = 10000000}, > > + { .name = "PPS1", .value = 0x0001, .frequency = 1 }, > > + { .name = "PPS2", .value = 0x0002, .frequency = 1 }, > > + { .name = "TS1", .value = 0x0004, .frequency = 0 }, > > + { .name = "TS2", .value = 0x0008, .frequency = 0 }, > > + { .name = "FREQ1", .value = 0x0100, .frequency = 0 }, > > + { .name = "None", .value = SMA_DISABLE, .frequency = 0 }, > > + { } > > +}; > > + > > +static const struct ocp_selector ptp_ocp_adva_sma_out[] = { > > + { .name = "10Mhz", .value = 0x0000, .frequency = 10000000}, > > + { .name = "PHC", .value = 0x0001, .frequency = 1 }, > > + { .name = "GNSS1", .value = 0x0004, .frequency = 1 }, > > + { .name = "GEN1", .value = 0x0040 }, > > + { .name = "GND", .value = 0x2000 }, > > + { .name = "VCC", .value = 0x4000 }, > > + { } > > +}; > > + > > struct ocp_sma_op { > > const struct ocp_selector *tbl[2]; > > void (*init)(struct ptp_ocp *bp); > > @@ -1363,20 +1512,20 @@ ptp_ocp_estimate_pci_timing(struct ptp_ocp *bp) > > } > > > > static int > > -ptp_ocp_init_clock(struct ptp_ocp *bp) > > +ptp_ocp_init_clock(struct ptp_ocp *bp, struct servo_val *servo_vals) > > { > > struct timespec64 ts; > > u32 ctrl; > > > > + > > no need for the second empty line > Will be fixed on patch V2 > > ctrl = OCP_CTRL_ENABLE; > > iowrite32(ctrl, &bp->reg->ctrl); > > > > - /* NO DRIFT Correction */ > > - /* offset_p:i 1/8, offset_i: 1/16, drift_p: 0, drift_i: 0 */ > > - iowrite32(0x2000, &bp->reg->servo_offset_p); > > - iowrite32(0x1000, &bp->reg->servo_offset_i); > > - iowrite32(0, &bp->reg->servo_drift_p); > > - iowrite32(0, &bp->reg->servo_drift_i); > > + /* servo configuration */ > > + iowrite32(servo_vals->servo_offset_p_val, &bp->reg->servo_offset_p); > > + iowrite32(servo_vals->servo_offset_i_val, &bp->reg->servo_offset_i); > > + iowrite32(servo_vals->servo_drift_p_val, &bp->reg->servo_drift_p); > > + iowrite32(servo_vals->servo_drift_p_val, &bp->reg->servo_drift_i); > > > > /* latch servo values */ > > ctrl |= OCP_CTRL_ADJUST_SERVO; > > @@ -2362,6 +2511,14 @@ static const struct ocp_sma_op ocp_fb_sma_op = { > > .set_output = ptp_ocp_sma_fb_set_output, > > }; > > > > +static const struct ocp_sma_op ocp_adva_sma_op = { > > + .tbl = { ptp_ocp_adva_sma_in, ptp_ocp_adva_sma_out }, > > + .init = ptp_ocp_sma_fb_init, > > + .get = ptp_ocp_sma_fb_get, > > + .set_inputs = ptp_ocp_sma_fb_set_inputs, > > + .set_output = ptp_ocp_sma_fb_set_output, > > +}; > > + > > static int > > ptp_ocp_set_pins(struct ptp_ocp *bp) > > { > > @@ -2420,6 +2577,7 @@ static int > > ptp_ocp_fb_board_init(struct ptp_ocp *bp, struct ocp_resource *r) > > { > > int err; > > + struct servo_val servo_vals; > > > > bp->flash_start = 1024 * 4096; > > bp->eeprom_map = fb_eeprom_map; > > @@ -2441,7 +2599,14 @@ ptp_ocp_fb_board_init(struct ptp_ocp *bp, struct ocp_resource *r) > > return err; > > ptp_ocp_sma_init(bp); > > > > - return ptp_ocp_init_clock(bp); > > + /* NO DRIFT Correction */ > > + /* offset_p:i 1/8, offset_i: 1/16, drift_p: 0, drift_i: 0 */ > > + servo_vals.servo_offset_p_val = 0x2000; > > + servo_vals.servo_offset_i_val = 0x1000; > > + servo_vals.servo_drift_p_val = 0; > > + servo_vals.servo_drift_p_val = 0; > > instead of adding this to every particular initialization function, > struct ptp_ocp_servo_conf can be put to .extra field of the resource > holding init function. This will move all configuration points to the > list of card-specific resources, the place to have differences of cards > and will make the code cleaner and eliminate all these local structs. > We can potentially create another static function to configure servo > part, but it's up to you. > Will be done on patch V2 > > + > > + return ptp_ocp_init_clock(bp, &servo_vals); > > } > > > > static bool > > @@ -2583,6 +2748,7 @@ static int > > ptp_ocp_art_board_init(struct ptp_ocp *bp, struct ocp_resource *r) > > { > > int err; > > + struct servo_val servo_vals; > > > > bp->flash_start = 0x1000000; > > bp->eeprom_map = art_eeprom_map; > > @@ -2603,7 +2769,49 @@ ptp_ocp_art_board_init(struct ptp_ocp *bp, struct ocp_resource *r) > > if (err) > > return err; > > > > - return ptp_ocp_init_clock(bp); > > + /* NO DRIFT Correction */ > > + /* offset_p:i 1/8, offset_i: 1/16, drift_p: 0, drift_i: 0 */ > > + servo_vals.servo_offset_p_val = 0x2000; > > + servo_vals.servo_offset_i_val = 0x1000; > > + servo_vals.servo_drift_p_val = 0; > > + servo_vals.servo_drift_p_val = 0; > > + > > + return ptp_ocp_init_clock(bp, &servo_vals); > > +} > > + > > +/* ADVA specific board initializers; last "resource" registered. */ > > +static int > > +ptp_ocp_adva_board_init(struct ptp_ocp *bp, struct ocp_resource *r) > > +{ > > + int err; > > + struct servo_val servo_vals; > > + > > + bp->flash_start = 0xA00000; > > + bp->fw_version = ioread32(&bp->image->version); > > + bp->sma_op = &ocp_adva_sma_op; > > + > > + ptp_ocp_fb_set_version(bp); > > + > > + ptp_ocp_tod_init(bp); > > + ptp_ocp_nmea_out_init(bp); > > + ptp_ocp_signal_init(bp); > > + > > + err = ptp_ocp_attr_group_add(bp, adva_timecard_groups); > > + if (err) > > + return err; > > + > > + err = ptp_ocp_set_pins(bp); > > + if (err) > > + return err; > > + ptp_ocp_sma_init(bp); > > + > > + /* offset_p:i 3/4, offset_i: 1/16, drift_p: 0, drift_i: 0 */ > > + servo_vals.servo_offset_p_val = 0xc000; > > + servo_vals.servo_offset_i_val = 0x1000; > > + servo_vals.servo_drift_p_val = 0; > > + servo_vals.servo_drift_p_val = 0; > > + > > + return ptp_ocp_init_clock(bp, &servo_vals); > > } > > > > static ssize_t > > @@ -3578,6 +3786,35 @@ static const struct ocp_attr_group art_timecard_groups[] = { > > { }, > > }; > > > > +static struct attribute *adva_timecard_attrs[] = { > > + &dev_attr_serialnum.attr, > > + &dev_attr_gnss_sync.attr, > > + &dev_attr_clock_source.attr, > > + &dev_attr_available_clock_sources.attr, > > + &dev_attr_sma1.attr, > > + &dev_attr_sma2.attr, > > + &dev_attr_sma3.attr, > > + &dev_attr_sma4.attr, > > + &dev_attr_available_sma_inputs.attr, > > + &dev_attr_available_sma_outputs.attr, > > + &dev_attr_clock_status_drift.attr, > > + &dev_attr_clock_status_offset.attr, > > + &dev_attr_ts_window_adjust.attr, > > + &dev_attr_tod_correction.attr, > > + NULL, > > +}; > > + > > +static const struct attribute_group adva_timecard_group = { > > + .attrs = adva_timecard_attrs, > > +}; > > + > > +static const struct ocp_attr_group adva_timecard_groups[] = { > > + { .cap = OCP_CAP_BASIC, .group = &adva_timecard_group }, > > + { .cap = OCP_CAP_SIGNAL, .group = &fb_timecard_signal0_group }, > > + { .cap = OCP_CAP_FREQ, .group = &fb_timecard_freq0_group }, > > + { }, > > +}; > > + > > static void > > gpio_input_map(char *buf, struct ptp_ocp *bp, u16 map[][2], u16 bit, > > const char *def) > > starting from here ... > > > @@ -4492,7 +4729,7 @@ ptp_ocp_remove(struct pci_dev *pdev) > > cancel_delayed_work_sync(&bp->sync_work); > > for (i = 0; i < OCP_SMA_NUM; i++) { > > if (bp->sma[i].dpll_pin) { > > - dpll_pin_unregister(bp->dpll, bp->sma[i].dpll_pin, &dpll_pins_ops, bp); > > + dpll_pin_unregister(bp->dpll, bp->sma[i].dpll_pin, &dpll_pins_ops, &bp->sma[i]); > > dpll_pin_put(bp->sma[i].dpll_pin); > > } > > } > > the chuck is already in a different patch and is reviewed actually, no > need to post it again. Will be fixed on patch V2
Powered by blists - more mailing lists