Re: [linux-sunxi][PATCH] ARM: dts: sun7i: Enable HDMI support on the Olimex EVB
On 31 January 2018 at 16:34, Maxime Ripard wrote: > On Tue, Jan 30, 2018 at 08:50:54AM +0100, codekip...@gmail.com wrote: >> From: Marcus Cooper >> >> Enable the display pipeline and HDMI output on the Olimex >> A20-SOM-EVB. >> >> Signed-off-by: Marcus Cooper >> --- >> arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts | 25 >> + >> 1 file changed, 25 insertions(+) >> >> diff --git a/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts >> b/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts >> index 64c8ef9a2756..07906d4f8758 100644 >> --- a/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts >> +++ b/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts >> @@ -61,6 +61,17 @@ >> stdout-path = "serial0:115200n8"; >> }; >> >> + hdmi-connector { >> + compatible = "hdmi-connector"; >> + type = "a"; >> + >> + port { >> + hdmi_con_in: endpoint { >> + remote-endpoint = <&hdmi_out_con>; >> + }; >> + }; >> + }; >> + >> leds { >> compatible = "gpio-leds"; >> pinctrl-names = "default"; >> @@ -79,6 +90,10 @@ >> status = "okay"; >> }; >> >> +&de { >> + status = "okay"; >> +}; >> + >> &ehci0 { >> status = "okay"; >> }; >> @@ -107,6 +122,16 @@ >> }; >> }; >> >> +&hdmi { >> + status = "okay"; >> +}; >> + >> +&hdmi_out { >> + hdmi_out_con: endpoint { >> + remote-endpoint = <&hdmi_con_in>; >> + }; >> +}; >> + > > The indentation is not correct here. ACK > > Maxime > > -- > Maxime Ripard, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[linux-sunxi] Re: [PATCH v2 07/16] iio: adc: sun4i-gpadc-iio: rework: support nvmem calibration data
Hi Philipp, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on robh/for-next] [also build test WARNING on v4.15 next-20180126] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Philipp-Rossak/IIO-based-thermal-sensor-driver-for-Allwinner-H3-and-A83T-SoC/20180201-043415 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) >> drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32 >> drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32 >> drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32 >> drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32 >> drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32 >> drivers/iio/adc/sun4i-gpadc-iio.c:608:53: sparse: cast to restricted __be32 drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32 drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32 drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32 drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32 drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32 drivers/iio/adc/sun4i-gpadc-iio.c:613:53: sparse: cast to restricted __be32 vim +608 drivers/iio/adc/sun4i-gpadc-iio.c 564 565 static int sun4i_gpadc_probe_dt(struct platform_device *pdev, 566 struct iio_dev *indio_dev) 567 { 568 struct sun4i_gpadc_iio *info = iio_priv(indio_dev); 569 struct resource *mem; 570 void __iomem *base; 571 int ret; 572 struct nvmem_cell *cell; 573 ssize_t cell_size; 574 u64 *cell_data; 575 576 info->data = of_device_get_match_data(&pdev->dev); 577 if (!info->data) 578 return -ENODEV; 579 580 info->no_irq = true; 581 indio_dev->num_channels = ARRAY_SIZE(sun8i_a33_gpadc_channels); 582 indio_dev->channels = sun8i_a33_gpadc_channels; 583 584 mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); 585 base = devm_ioremap_resource(&pdev->dev, mem); 586 if (IS_ERR(base)) 587 return PTR_ERR(base); 588 589 info->has_calibration_data[0] = false; 590 info->has_calibration_data[1] = false; 591 592 if (!info->data->supports_nvmem) 593 goto no_nvmem; 594 595 cell = nvmem_cell_get(&pdev->dev, "calibration"); 596 if (IS_ERR(cell)) { 597 if (PTR_ERR(cell) == -EPROBE_DEFER) 598 return PTR_ERR(cell); 599 goto no_nvmem; 600 } 601 602 cell_data = (u64 *)nvmem_cell_read(cell, &cell_size); 603 nvmem_cell_put(cell); 604 switch (cell_size) { 605 case 8: 606 case 6: 607 info->has_calibration_data[1] = true; > 608 info->calibration_data[1] = be32_to_cpu( 609 upper_32_bits(cell_data[0])); 610 case 4: 611 case 2: 612 info->has_calibration_data[0] = true; 613 info->calibration_data[0] = be32_to_cpu( 614 lower_32_bits(cell_data[0])); 615 break; 616 default: 617 break; 618 } 619 620 no_nvmem: 621 622 info->regmap = devm_regmap_init_mmio(&pdev->dev, base, 623 &sun4i_gpadc_regmap_config); 624 if (IS_ERR(info->regmap)) { 625 ret = PTR_ERR(info->regmap); 626 dev_err(&pdev->dev, "failed to init regmap: %d\n", ret); 627 return ret; 628 } 629 630 if (info->data->has_bus_rst) { 631 info->reset = devm_reset_control_get(&pdev->dev, NULL); 632 if (IS_ERR(info->reset)) { 633 ret = PTR_ERR(info->reset); 634 return ret; 635 } 636 637 ret = reset_control_deassert(info->reset); 638 if (ret) 639 return ret; 640 } 641 642 if (info->data->has_bus_clk) { 643 info->bus_clk = devm_
[linux-sunxi] Re: [PATCH v2 08/16] iio: adc: sun4i-gpadc-iio: rework: add interrupt support
Hi Philipp, Thank you for the patch! Yet something to improve: [auto build test ERROR on robh/for-next] [also build test ERROR on v4.15 next-20180126] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Philipp-Rossak/IIO-based-thermal-sensor-driver-for-Allwinner-H3-and-A83T-SoC/20180201-043415 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next config: xtensa-allmodconfig (attached as .config) compiler: xtensa-linux-gcc (GCC) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=xtensa Note: the linux-review/Philipp-Rossak/IIO-based-thermal-sensor-driver-for-Allwinner-H3-and-A83T-SoC/20180201-043415 HEAD e571c3ced84a9d7f150cb2d239919d31d25114e2 builds fine. It only hurts bisectibility. All errors (new ones prefixed by >>): drivers/iio/adc/sun4i-gpadc-iio.c: In function 'sunxi_irq_thread': >> drivers/iio/adc/sun4i-gpadc-iio.c:448:29: error: 'SUN8I_H3_THS_STAT' >> undeclared (first use in this function); did you mean 'SUN8I_H3_THS_INTC'? regmap_write(info->regmap, SUN8I_H3_THS_STAT, info->data->irq_clear_map); ^ SUN8I_H3_THS_INTC drivers/iio/adc/sun4i-gpadc-iio.c:448:29: note: each undeclared identifier is reported only once for each function it appears in vim +448 drivers/iio/adc/sun4i-gpadc-iio.c 443 444 static irqreturn_t sunxi_irq_thread(int irq, void *data) 445 { 446 struct sun4i_gpadc_iio *info = data; 447 > 448 regmap_write(info->regmap, SUN8I_H3_THS_STAT, > info->data->irq_clear_map); 449 450 thermal_zone_device_update(info->tzd, THERMAL_EVENT_TEMP_SAMPLE); 451 452 return IRQ_HANDLED; 453 } 454 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout. .config.gz Description: application/gzip
[linux-sunxi] Re: [PATCH v2 09/16] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor
Hi Philipp, On Mon, Jan 29, 2018 at 12:29:12AM +0100, Philipp Rossak wrote: > This patch adds support for the H3 ths sensor. > > The H3 supports interrupts. The interrupt is configured to update the > the sensor values every second. The calibration data is writen at the > begin of the init process. > > Signed-off-by: Philipp Rossak > --- > drivers/iio/adc/sun4i-gpadc-iio.c | 86 > +++ > include/linux/mfd/sun4i-gpadc.h | 22 ++ > 2 files changed, 108 insertions(+) > > diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c > b/drivers/iio/adc/sun4i-gpadc-iio.c > index b7b5451226b0..8196203d65fe 100644 > --- a/drivers/iio/adc/sun4i-gpadc-iio.c > +++ b/drivers/iio/adc/sun4i-gpadc-iio.c > @@ -61,6 +61,9 @@ struct sun4i_gpadc_iio; > static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info); > static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info); > > +static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info); > +static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info); > + We try to avoid using the generic sunxi prefix. > struct gpadc_data { > int temp_offset; > int temp_scale; > @@ -71,6 +74,10 @@ struct gpadc_data { > unsigned inttemp_data[MAX_SENSOR_COUNT]; > int (*sample_start)(struct sun4i_gpadc_iio *info); > int (*sample_end)(struct sun4i_gpadc_iio *info); > + u32 ctrl0_map; > + u32 ctrl2_map; > + u32 sensor_en_map; > + u32 filter_map; > u32 irq_clear_map; > u32 irq_control_map; > boolhas_bus_clk; > @@ -138,6 +145,31 @@ static const struct gpadc_data sun8i_a33_gpadc_data = { > .support_irq = false, > }; > > +static const struct gpadc_data sun8i_h3_ths_data = { > + .temp_offset = -1791, > + .temp_scale = -121, > + .temp_data = {SUN8I_H3_THS_TDATA0, 0, 0, 0}, > + .sample_start = sunxi_ths_sample_start, > + .sample_end = sunxi_ths_sample_end, > + .has_bus_clk = true, > + .has_bus_rst = true, > + .has_mod_clk = true, > + .sensor_count = 1, > + .supports_nvmem = true, > + .support_irq = true, > + .ctrl0_map = SUN4I_GPADC_CTRL0_T_ACQ(0xff), > + .ctrl2_map = SUN8I_H3_THS_ACQ1(0x3f), > + .sensor_en_map = SUN8I_H3_THS_TEMP_SENSE_EN0, > + .filter_map = SUN4I_GPADC_CTRL3_FILTER_EN | > + SUN4I_GPADC_CTRL3_FILTER_TYPE(0x2), > + .irq_clear_map = SUN8I_H3_THS_INTS_ALARM_INT_0 | > + SUN8I_H3_THS_INTS_SHUT_INT_0 | > + SUN8I_H3_THS_INTS_TDATA_IRQ_0 | > + SUN8I_H3_THS_INTS_ALARM_OFF_0, > + .irq_control_map = SUN8I_H3_THS_INTC_TDATA_IRQ_EN0 | > + SUN8I_H3_THS_TEMP_PERIOD(0x7), >From what I've understood, ACQ regs are basically clock dividers. We should make a better job at explaining it :) > +}; > + > struct sun4i_gpadc_iio { > struct iio_dev *indio_dev; > struct completion completion; > @@ -462,6 +494,16 @@ static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio > *info) > return 0; > } > > +static int sunxi_ths_sample_end(struct sun4i_gpadc_iio *info) > +{ > + /* Disable ths interrupt */ > + regmap_write(info->regmap, SUN8I_H3_THS_INTC, 0x0); > + /* Disable temperature sensor */ > + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, 0x0); > + > + return 0; > +} > + > static int sun4i_gpadc_runtime_suspend(struct device *dev) > { > struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev)); > @@ -473,6 +515,17 @@ static int sun4i_gpadc_runtime_suspend(struct device > *dev) > return info->data->sample_end(info); > } > > +static void sunxi_calibrate(struct sun4i_gpadc_iio *info) > +{ > + if (info->has_calibration_data[0]) > + regmap_write(info->regmap, SUNXI_THS_CDATA_0_1, > + info->calibration_data[0]); > + > + if (info->has_calibration_data[1]) > + regmap_write(info->regmap, SUNXI_THS_CDATA_2_3, > + info->calibration_data[1]); > +} > + > static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info) > { > /* clkin = 6MHz */ > @@ -492,6 +545,35 @@ static int sun4i_gpadc_sample_start(struct > sun4i_gpadc_iio *info) > return 0; > } > > +static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info) > +{ > + u32 value; > + sunxi_calibrate(info); > + > + if (info->data->ctrl0_map) > + regmap_write(info->regmap, SUN8I_H3_THS_CTRL0, > + info->data->ctrl0_map); > + > + regmap_write(info->regmap, SUN8I_H3_THS_CTRL2, > + info->data->ctrl2_map); > + > + regmap_write(info->regmap, SUN8I_H3_THS_STAT, > + info->data->irq_clear_map); > + > + regmap_write(info->regmap, SUN8I_H3_THS_FILTER, > + info->data->filter_map); >
[linux-sunxi] Re: [PATCH v2 08/16] iio: adc: sun4i-gpadc-iio: rework: add interrupt support
Hi Philipp, On Mon, Jan 29, 2018 at 12:29:11AM +0100, Philipp Rossak wrote: > This patch rewors the driver to support interrupts for the thermal part > of the sensor. > > This is only available for the newer sensor (currently H3 and A83T). > The interrupt will be trigerd on data available and triggers the update > for the thermal sensors. All newer sensors have different amount of > sensors and different interrupts for each device the reset of the > interrupts need to be done different > > For the newer sensors is the autosuspend disabled. > > Signed-off-by: Philipp Rossak > Acked-by: Jonathan Cameron > --- > drivers/iio/adc/sun4i-gpadc-iio.c | 60 > +++ > include/linux/mfd/sun4i-gpadc.h | 2 ++ > 2 files changed, 56 insertions(+), 6 deletions(-) > > diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c > b/drivers/iio/adc/sun4i-gpadc-iio.c > index 74eeb5cd5218..b7b5451226b0 100644 > --- a/drivers/iio/adc/sun4i-gpadc-iio.c > +++ b/drivers/iio/adc/sun4i-gpadc-iio.c > @@ -71,11 +71,14 @@ struct gpadc_data { > unsigned inttemp_data[MAX_SENSOR_COUNT]; > int (*sample_start)(struct sun4i_gpadc_iio *info); > int (*sample_end)(struct sun4i_gpadc_iio *info); > + u32 irq_clear_map; > + u32 irq_control_map; I would say to use a regmap_irq_chip for controlling IRQs. > boolhas_bus_clk; > boolhas_bus_rst; > boolhas_mod_clk; > int sensor_count; > boolsupports_nvmem; > + boolsupport_irq; > }; > > static const struct gpadc_data sun4i_gpadc_data = { > @@ -90,6 +93,7 @@ static const struct gpadc_data sun4i_gpadc_data = { > .sample_end = sun4i_gpadc_sample_end, > .sensor_count = 1, > .supports_nvmem = false, > + .support_irq = false, False is the default, no need to set support_irq. [...] > struct sun4i_gpadc_iio { > @@ -332,6 +339,11 @@ static int sun4i_gpadc_temp_read(struct iio_dev > *indio_dev, int *val, > return 0; > } > > + if (info->data->support_irq) { > + regmap_read(info->regmap, info->data->temp_data[sensor], val); > + return 0; > + } > + Maybe you could define a new thermal_zone_of_device_ops for these new thermal sensors? That way, you don't even need the boolean support_irq. > return sun4i_gpadc_read(indio_dev, 0, val, info->temp_data_irq); > } > > @@ -429,6 +441,17 @@ static irqreturn_t sun4i_gpadc_fifo_data_irq_handler(int > irq, void *dev_id) > return IRQ_HANDLED; > } > > +static irqreturn_t sunxi_irq_thread(int irq, void *data) I think we're trying to avoid sunxi mentions but rather using the name of the first IP (in term of product release, not support) using this function. > +{ > + struct sun4i_gpadc_iio *info = data; > + > + regmap_write(info->regmap, SUN8I_H3_THS_STAT, > info->data->irq_clear_map); > + Will be handled by regmap_irq_chip. [...] > - info->no_irq = true; > + if (info->data->support_irq) { > + /* only the new versions of ths support right now irqs */ > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) { > + dev_err(&pdev->dev, "failed to get IRQ: %d\n", irq); > + return irq; > + } > + > + ret = devm_request_threaded_irq(&pdev->dev, irq, NULL, > + sunxi_irq_thread, IRQF_ONESHOT, > + dev_name(&pdev->dev), info); > + if (ret) > + return ret; > + > + } else > + info->no_irq = true; > + That's a bit funny to have two booleans named no_irq and support_irq :) > indio_dev->num_channels = ARRAY_SIZE(sun8i_a33_gpadc_channels); > indio_dev->channels = sun8i_a33_gpadc_channels; > > @@ -789,11 +829,13 @@ static int sun4i_gpadc_probe(struct platform_device > *pdev) > if (ret) > return ret; > > - pm_runtime_set_autosuspend_delay(&pdev->dev, > - SUN4I_GPADC_AUTOSUSPEND_DELAY); > - pm_runtime_use_autosuspend(&pdev->dev); > - pm_runtime_set_suspended(&pdev->dev); > - pm_runtime_enable(&pdev->dev); > + if (!info->data->support_irq) { > + pm_runtime_set_autosuspend_delay(&pdev->dev, > + SUN4I_GPADC_AUTOSUSPEND_DELAY); > + pm_runtime_use_autosuspend(&pdev->dev); > + pm_runtime_set_suspended(&pdev->dev); > + pm_runtime_enable(&pdev->dev); > + } Why would you not want your IP to do runtime PM? Quentin -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com
[linux-sunxi] Re: [PATCH v2 06/16] iio: adc: sun4i-gpadc-iio: rework: support multiple sensors
Hi Philipp, On Mon, Jan 29, 2018 at 12:29:09AM +0100, Philipp Rossak wrote: > For adding newer sensor some basic rework of the code is necessary. > > This patch reworks the driver to be able to handle more than one > thermal sensor. Newer SoC like the A80 have 4 thermal sensors. > Because of this the maximal sensor count value was set to 4. > > The sensor_id value is set during sensor registration and is for each > registered sensor indiviual. This makes it able to differntiate the > sensors when the value is read from the register. > > In function sun4i_gpadc_read_raw(), the sensor number of the ths sensor > was directly set to 0 (sun4i_gpadc_temp_read(x,x,0)). This selects > in the temp_read function automatically sensor 0. A check for the > sensor_id is here not required since the old sensors only have one > thermal sensor. In addition to that is the sun4i_gpadc_read_raw() > function only used by the "older" sensors (before A33) where the > thermal sensor was a cobination of an adc and a thermal sensor. > > Signed-off-by: Philipp Rossak > --- > drivers/iio/adc/sun4i-gpadc-iio.c | 36 +++- > include/linux/mfd/sun4i-gpadc.h | 3 +++ > 2 files changed, 26 insertions(+), 13 deletions(-) > > diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c > b/drivers/iio/adc/sun4i-gpadc-iio.c > index 51ec0104d678..ac9ad2f8232f 100644 > --- a/drivers/iio/adc/sun4i-gpadc-iio.c > +++ b/drivers/iio/adc/sun4i-gpadc-iio.c > @@ -67,12 +67,13 @@ struct gpadc_data { > unsigned inttp_adc_select; > unsigned int(*adc_chan_select)(unsigned int chan); > unsigned intadc_chan_mask; > - unsigned inttemp_data; > + unsigned inttemp_data[MAX_SENSOR_COUNT]; > int (*sample_start)(struct sun4i_gpadc_iio *info); > int (*sample_end)(struct sun4i_gpadc_iio *info); > boolhas_bus_clk; > boolhas_bus_rst; > boolhas_mod_clk; > + int sensor_count; > }; > I've noticed that for H3, A83T, A64 (at least), if DATA reg of sensor 0 is e.g. 0x80, DATA reg of sensor N is at 0x80 + 0x04 * N. Is that verified for other SoCs? Does anyone have some input on this? We could then just use temp_data as the DATA reg "base" and increment by 0x4 depending on the sensor id instead of using a fixed-size array. > static const struct gpadc_data sun4i_gpadc_data = { > @@ -82,9 +83,10 @@ static const struct gpadc_data sun4i_gpadc_data = { > .tp_adc_select = SUN4I_GPADC_CTRL1_TP_ADC_SELECT, > .adc_chan_select = &sun4i_gpadc_chan_select, > .adc_chan_mask = SUN4I_GPADC_CTRL1_ADC_CHAN_MASK, > - .temp_data = SUN4I_GPADC_TEMP_DATA, > + .temp_data = {SUN4I_GPADC_TEMP_DATA, 0, 0, 0}, > .sample_start = sun4i_gpadc_sample_start, > .sample_end = sun4i_gpadc_sample_end, > + .sensor_count = 1, If the solution above is not desirable/possible, could we use something like: unsigned int sun4i_temp_data[] = {SUN4I_GPADC_TEMP_DATA,}; static const struct gpadc_data sun4i_gpadc_data = { .temp_data = &sun4i_temp_data, .sensor_count = ARRAY_SIZE(sun4i_temp_data), }; That avoids 1) inconsistencies between the array size and the array itself, 2) does not require to pad the array with zeroes. [...] > @@ -745,9 +752,12 @@ static int sun4i_gpadc_probe(struct platform_device > *pdev) > pm_runtime_enable(&pdev->dev); > > if (IS_ENABLED(CONFIG_THERMAL_OF)) { > - info->tzd = thermal_zone_of_sensor_register(info->sensor_device, > - 0, info, > - &sun4i_ts_tz_ops); > + for (i = 0; i < info->data->sensor_count; i++) { > + info->sensor_id = i; > + info->tzd = thermal_zone_of_sensor_register( > + info->sensor_device, > + i, info, &sun4i_ts_tz_ops); > + } As Maxime said, this does not work. One way would be to have a new structure being: struct sun4i_sensor_info { struct sun4i_gpadc_iio *info; unsigned intsensor_id; }; Or since we only use the iio_dev within the sun4i_gpadc_iio in the .get_temp function, we may replace info by struct iio_dev *indio_dev above. Quentin -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout. signature.asc Description: PGP signature
[linux-sunxi] Re: [PATCH v2 04/16] iio: adc: sun4i-gpadc-iio: rework: sampling start/end code readout reg
On 31.01.2018 18:51, Quentin Schulz wrote: Hi Philipp, On Mon, Jan 29, 2018 at 12:29:07AM +0100, Philipp Rossak wrote: For adding newer sensor some basic rework of the code is necessary. This commit reworks the code and allows the sampling start/end code and the position of value readout register to be altered. Later the start/end functions will be used to configure the ths and start/stop the sampling. Signed-off-by: Icenowy Zheng Signed-off-by: Philipp Rossak --- drivers/iio/adc/sun4i-gpadc-iio.c | 44 ++- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c index 03804ff9c006..db57d9fffe48 100644 --- a/drivers/iio/adc/sun4i-gpadc-iio.c +++ b/drivers/iio/adc/sun4i-gpadc-iio.c @@ -49,6 +49,15 @@ static unsigned int sun6i_gpadc_chan_select(unsigned int chan) return SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(chan); } +struct sun4i_gpadc_iio; + +/* + * Prototypes for these functions, which enable these functions to be + * referenced in gpadc_data structures. + */ Comment not needed. +static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info); +static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info); + struct gpadc_data { int temp_offset; int temp_scale; @@ -56,6 +65,9 @@ struct gpadc_data { unsigned inttp_adc_select; unsigned int(*adc_chan_select)(unsigned int chan); unsigned intadc_chan_mask; + unsigned inttemp_data; Does not really have anything to do with sample_start/end. I would have made a different commit for it. Otherwise, Reviewed-by: Quentin Schulz Quentin Ok I will split this. Thanks, Philipp -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[linux-sunxi] Re: [PATCH v2 01/16] dt-bindings: update the Allwinner GPADC device tree binding for H3 & A83T
On 31.01.2018 18:40, Quentin Schulz wrote: Hi Philipp, On Mon, Jan 29, 2018 at 12:29:04AM +0100, Philipp Rossak wrote: Allwinner H3 features a thermal sensor like the one in A33, but has its register re-arranged, the clock divider moved to CCU (originally the clock divider is in ADC) and added a pair of bus clock and reset. Allwinner A83T features a thermal sensor similar to the H3, the ths clock, the bus clock and the reset was removed from the CCU. The THS in A83T has a clock that is directly connected and runs with 24 MHz. Update the binding document to cover H3 and A83T. Signed-off-by: Philipp Rossak --- .../devicetree/bindings/mfd/sun4i-gpadc.txt| 50 -- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt index 86dd8191b04c..22df0c5c23d4 100644 --- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt +++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt @@ -4,12 +4,35 @@ The Allwinner SoCs all have an ADC that can also act as a thermal sensor and sometimes as a touchscreen controller. Required properties: - - compatible: "allwinner,sun8i-a33-ths", + - compatible: must contain one of the following compatibles: + - "allwinner,sun8i-a33-ths" + - "allwinner,sun8i-h3-ths" + - "allwinner,sun8i-a83t-ths" - reg: mmio address range of the chip, - - #thermal-sensor-cells: shall be 0, + - #thermal-sensor-cells: shall be 0 or 1, Well, thermal-sensor-cells is either 0 or 1 :) Better to point to the documentation describing this thermal-sensor-cells IMHO. I agree, I will change this in the next version. - #io-channel-cells: shall be 0, -Example: +Required properties for the following compatibles: + - "allwinner,sun8i-h3-ths" + - "allwinner,sun8i-a83t-ths" + - interrupts: the sampling interrupt of the ADC, + +Required properties for the following compatibles: + - "allwinner,sun8i-h3-ths" + - clocks: the bus clock and the input clock of the ADC, + - clock-names: should be "bus" and "mod", + - resets: the bus reset of the ADC, + +Optional properties for the following compatibles: + - "allwinner,sun8i-h3-ths" + - nvmem-cells: A phandle to the calibration data provided by a nvmem device. + If unspecified default values shall be used. The size should + be 0x2 * sensorcount. "twice the number of sensors" ? As already mentioned in an other answers, this here is not correct. I got somehow a wrong information or mixed something up. For H5, H3, A83T and A64 the thermal sensor calibration data is always 64 bit wide and placed on the eFuse address 0x34 [1]. + - nvmem-cell-names: Should be "calibration". + +Details see: bindings/nvmem/nvmem.txt + +Example for A33: ths: ths@1c25000 { compatible = "allwinner,sun8i-a33-ths"; reg = <0x01c25000 0x100>; @@ -17,6 +40,27 @@ Example: #io-channel-cells = <0>; }; +Example for H3: + ths: thermal-sensor@1c25000 { + compatible = "allwinner,sun8i-h3-ths"; + reg = <0x01c25000 0x400>; + clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>; + clock-names = "bus", "mod"; + resets = <&ccu RST_BUS_THS>; + interrupts = ; + #thermal-sensor-cells = <0>; + #io-channel-cells = <0>; + }; + +Example for A83T: + ths: thermal-sensor@1f04000 { + compatible = "allwinner,sun8i-a83t-ths"; + reg = <0x01f04000 0x100>; + interrupts = ; + #thermal-sensor-cells = <1>; + #io-channel-cells = <0>; + }; + Aside from Maxime's comment on how we would like to refactor GPADC/THS, I'm not sure we really want an example for each an every thermal sensor supported. Quentin I agree we don't want to have an example for every sensor, but I think at least those two are interesting, since one has 3 sensors and no clocks, and one has 1 sensor and clocks. Thanks, Philipp [1]: http://linux-sunxi.org/SID_Register_Guide#eFUSE -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[linux-sunxi] Re: [PATCH v2 04/16] iio: adc: sun4i-gpadc-iio: rework: sampling start/end code readout reg
Hi Philipp, On Mon, Jan 29, 2018 at 12:29:07AM +0100, Philipp Rossak wrote: > For adding newer sensor some basic rework of the code is necessary. > > This commit reworks the code and allows the sampling start/end code and > the position of value readout register to be altered. Later the start/end > functions will be used to configure the ths and start/stop the > sampling. > > Signed-off-by: Icenowy Zheng > Signed-off-by: Philipp Rossak > --- > drivers/iio/adc/sun4i-gpadc-iio.c | 44 > ++- > 1 file changed, 39 insertions(+), 5 deletions(-) > > diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c > b/drivers/iio/adc/sun4i-gpadc-iio.c > index 03804ff9c006..db57d9fffe48 100644 > --- a/drivers/iio/adc/sun4i-gpadc-iio.c > +++ b/drivers/iio/adc/sun4i-gpadc-iio.c > @@ -49,6 +49,15 @@ static unsigned int sun6i_gpadc_chan_select(unsigned int > chan) > return SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(chan); > } > > +struct sun4i_gpadc_iio; > + > +/* > + * Prototypes for these functions, which enable these functions to be > + * referenced in gpadc_data structures. > + */ Comment not needed. > +static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info); > +static int sun4i_gpadc_sample_end(struct sun4i_gpadc_iio *info); > + > struct gpadc_data { > int temp_offset; > int temp_scale; > @@ -56,6 +65,9 @@ struct gpadc_data { > unsigned inttp_adc_select; > unsigned int(*adc_chan_select)(unsigned int chan); > unsigned intadc_chan_mask; > + unsigned inttemp_data; Does not really have anything to do with sample_start/end. I would have made a different commit for it. Otherwise, Reviewed-by: Quentin Schulz Quentin -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout. signature.asc Description: PGP signature
[linux-sunxi] Re: [PATCH v2 01/16] dt-bindings: update the Allwinner GPADC device tree binding for H3 & A83T
Hi Philipp, On Mon, Jan 29, 2018 at 12:29:04AM +0100, Philipp Rossak wrote: > Allwinner H3 features a thermal sensor like the one in A33, but has its > register re-arranged, the clock divider moved to CCU (originally the > clock divider is in ADC) and added a pair of bus clock and reset. > > Allwinner A83T features a thermal sensor similar to the H3, the ths clock, > the bus clock and the reset was removed from the CCU. The THS in A83T > has a clock that is directly connected and runs with 24 MHz. > > Update the binding document to cover H3 and A83T. > > Signed-off-by: Philipp Rossak > --- > .../devicetree/bindings/mfd/sun4i-gpadc.txt| 50 > -- > 1 file changed, 47 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt > b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt > index 86dd8191b04c..22df0c5c23d4 100644 > --- a/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt > +++ b/Documentation/devicetree/bindings/mfd/sun4i-gpadc.txt > @@ -4,12 +4,35 @@ The Allwinner SoCs all have an ADC that can also act as a > thermal sensor > and sometimes as a touchscreen controller. > > Required properties: > - - compatible: "allwinner,sun8i-a33-ths", > + - compatible: must contain one of the following compatibles: > + - "allwinner,sun8i-a33-ths" > + - "allwinner,sun8i-h3-ths" > + - "allwinner,sun8i-a83t-ths" >- reg: mmio address range of the chip, > - - #thermal-sensor-cells: shall be 0, > + - #thermal-sensor-cells: shall be 0 or 1, Well, thermal-sensor-cells is either 0 or 1 :) Better to point to the documentation describing this thermal-sensor-cells IMHO. >- #io-channel-cells: shall be 0, > > -Example: > +Required properties for the following compatibles: > + - "allwinner,sun8i-h3-ths" > + - "allwinner,sun8i-a83t-ths" > + - interrupts: the sampling interrupt of the ADC, > + > +Required properties for the following compatibles: > + - "allwinner,sun8i-h3-ths" > + - clocks: the bus clock and the input clock of the ADC, > + - clock-names: should be "bus" and "mod", > + - resets: the bus reset of the ADC, > + > +Optional properties for the following compatibles: > + - "allwinner,sun8i-h3-ths" > + - nvmem-cells: A phandle to the calibration data provided by a nvmem > device. > + If unspecified default values shall be used. The size should > + be 0x2 * sensorcount. "twice the number of sensors" ? > + - nvmem-cell-names: Should be "calibration". > + > +Details see: bindings/nvmem/nvmem.txt > + > +Example for A33: > ths: ths@1c25000 { > compatible = "allwinner,sun8i-a33-ths"; > reg = <0x01c25000 0x100>; > @@ -17,6 +40,27 @@ Example: > #io-channel-cells = <0>; > }; > > +Example for H3: > + ths: thermal-sensor@1c25000 { > + compatible = "allwinner,sun8i-h3-ths"; > + reg = <0x01c25000 0x400>; > + clocks = <&ccu CLK_BUS_THS>, <&ccu CLK_THS>; > + clock-names = "bus", "mod"; > + resets = <&ccu RST_BUS_THS>; > + interrupts = ; > + #thermal-sensor-cells = <0>; > + #io-channel-cells = <0>; > + }; > + > +Example for A83T: > + ths: thermal-sensor@1f04000 { > + compatible = "allwinner,sun8i-a83t-ths"; > + reg = <0x01f04000 0x100>; > + interrupts = ; > + #thermal-sensor-cells = <1>; > + #io-channel-cells = <0>; > + }; > + Aside from Maxime's comment on how we would like to refactor GPADC/THS, I'm not sure we really want an example for each an every thermal sensor supported. Quentin -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout. signature.asc Description: PGP signature
Re: [linux-sunxi][PATCH] ARM: sun6i: a31: Enable HDMI support on the Mele I7
On 31 January 2018 at 16:32, Maxime Ripard wrote: > On Tue, Jan 30, 2018 at 07:33:27PM +0100, codekip...@gmail.com wrote: >> From: Marcus Cooper >> >> The Mele I7 has an HDMI connector wired to the HDMI pins >> on the SoC. Enable the display pipeline and HDMI output. >> >> Signed-off-by: Marcus Cooper > > Chen-Yu is missing from the recipients... ACK...I'll update my git config for this repo. > >> --- >> arch/arm/boot/dts/sun6i-a31-i7.dts | 29 + >> 1 file changed, 29 insertions(+) >> >> diff --git a/arch/arm/boot/dts/sun6i-a31-i7.dts >> b/arch/arm/boot/dts/sun6i-a31-i7.dts >> index 010a84c7c012..4f69be4988a5 100644 >> --- a/arch/arm/boot/dts/sun6i-a31-i7.dts >> +++ b/arch/arm/boot/dts/sun6i-a31-i7.dts >> @@ -58,6 +58,17 @@ >> stdout-path = "serial0:115200n8"; >> }; >> >> + hdmi-connector { >> + compatible = "hdmi-connector"; >> + type = "a"; >> + >> + port { >> + hdmi_con_in: endpoint { >> + remote-endpoint = <&hdmi_out_con>; >> + }; >> + }; >> + }; >> + >> leds { >> compatible = "gpio-leds"; >> pinctrl-names = "default"; >> @@ -93,6 +104,10 @@ >> status = "okay"; >> }; >> >> +&de { >> + status = "okay"; >> +}; >> + >> &ehci0 { >> status = "okay"; >> }; >> @@ -101,6 +116,16 @@ >> status = "okay"; >> }; >> >> +&hdmi { >> + status = "okay"; >> +}; >> + >> +&hdmi_out { >> + hdmi_out_con: endpoint { >> + remote-endpoint = <&hdmi_con_in>; >> + }; >> +}; >> + >> &gmac { > > And this is not ordered alphabetically. ACKarrgghhh Will fix and repush soon. BR, CK > > maxime > > -- > Maxime Ripard, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: [linux-sunxi][PATCH] ARM: dts: sunxi: h3/h5: Add DAI node for HDMI
On Wed, Jan 31, 2018 at 10:54:29AM +0100, Code Kipper wrote: > On 31 January 2018 at 08:16, maxime ripard > wrote: > > On Mon, Jan 29, 2018 at 11:35:27AM +0100, Jernej Škrabec wrote: > >> Hi Maxime, > >> > >> (previously I respond only to linux-sunxi mailing list) > >> > >> >On Mon, Jan 29, 2018 at 10:22:23AM +0100, codekip...@gmail.com wrote: > >> >> From: Marcus Cooper > >> >> > >> >> Add the new DAI block for I2S2 which is used for HDMI audio. > >> >> > >> >> Signed-off-by: Marcus Cooper > >> > > >> >queued for 4.17, thanks! > >> >Maxime > >> > >> Please note that HDMI I2S has usable 4 I2S lanes, since HDMI > >> supports 8 channel audio. As Marcus said, other blocks probably > >> support them too, they are just not wired out on pins. > > > > I've dropped those patches for now. > > > >> Should we change compatible for HDMI? > > > > I guess, another way of doing things if they are strictly identical > > but for the number of lanes they support would be to add a DT property > > for that number of lanes. > > > That's fine...I'll look into adding a dt property and how we would map > channels to lanes. > Do you know of any examples?, Grepping for of_property_read_u32 should give you plenty of examples :) maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout. signature.asc Description: PGP signature
Re: [linux-sunxi][PATCH] ARM: dts: sun7i: Enable HDMI support on the Olimex EVB
On Tue, Jan 30, 2018 at 08:50:54AM +0100, codekip...@gmail.com wrote: > From: Marcus Cooper > > Enable the display pipeline and HDMI output on the Olimex > A20-SOM-EVB. > > Signed-off-by: Marcus Cooper > --- > arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts | 25 + > 1 file changed, 25 insertions(+) > > diff --git a/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts > b/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts > index 64c8ef9a2756..07906d4f8758 100644 > --- a/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts > +++ b/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts > @@ -61,6 +61,17 @@ > stdout-path = "serial0:115200n8"; > }; > > + hdmi-connector { > + compatible = "hdmi-connector"; > + type = "a"; > + > + port { > + hdmi_con_in: endpoint { > + remote-endpoint = <&hdmi_out_con>; > + }; > + }; > + }; > + > leds { > compatible = "gpio-leds"; > pinctrl-names = "default"; > @@ -79,6 +90,10 @@ > status = "okay"; > }; > > +&de { > + status = "okay"; > +}; > + > &ehci0 { > status = "okay"; > }; > @@ -107,6 +122,16 @@ > }; > }; > > +&hdmi { > + status = "okay"; > +}; > + > +&hdmi_out { > + hdmi_out_con: endpoint { > + remote-endpoint = <&hdmi_con_in>; > + }; > +}; > + The indentation is not correct here. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout. signature.asc Description: PGP signature
[linux-sunxi] Re: [PATCH v2 2/2] ARM: dts: sunxi: Add Olimex A20-SOM204-EVB-eMMC board
On Mon, Jan 29, 2018 at 03:56:40PM +0200, Stefan Mavrodiev wrote: > A20-SOM204 board has option with onboard 16GB eMMC. > The chip is wired to MMC2 slot. > > Signed-off-by: Stefan Mavrodiev Queued both in 4.17, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout. signature.asc Description: PGP signature
Re: [linux-sunxi][PATCH] ARM: sun6i: a31: Enable HDMI support on the Mele I7
On Tue, Jan 30, 2018 at 07:33:27PM +0100, codekip...@gmail.com wrote: > From: Marcus Cooper > > The Mele I7 has an HDMI connector wired to the HDMI pins > on the SoC. Enable the display pipeline and HDMI output. > > Signed-off-by: Marcus Cooper Chen-Yu is missing from the recipients... > --- > arch/arm/boot/dts/sun6i-a31-i7.dts | 29 + > 1 file changed, 29 insertions(+) > > diff --git a/arch/arm/boot/dts/sun6i-a31-i7.dts > b/arch/arm/boot/dts/sun6i-a31-i7.dts > index 010a84c7c012..4f69be4988a5 100644 > --- a/arch/arm/boot/dts/sun6i-a31-i7.dts > +++ b/arch/arm/boot/dts/sun6i-a31-i7.dts > @@ -58,6 +58,17 @@ > stdout-path = "serial0:115200n8"; > }; > > + hdmi-connector { > + compatible = "hdmi-connector"; > + type = "a"; > + > + port { > + hdmi_con_in: endpoint { > + remote-endpoint = <&hdmi_out_con>; > + }; > + }; > + }; > + > leds { > compatible = "gpio-leds"; > pinctrl-names = "default"; > @@ -93,6 +104,10 @@ > status = "okay"; > }; > > +&de { > + status = "okay"; > +}; > + > &ehci0 { > status = "okay"; > }; > @@ -101,6 +116,16 @@ > status = "okay"; > }; > > +&hdmi { > + status = "okay"; > +}; > + > +&hdmi_out { > + hdmi_out_con: endpoint { > + remote-endpoint = <&hdmi_con_in>; > + }; > +}; > + > &gmac { And this is not ordered alphabetically. maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout. signature.asc Description: PGP signature
Re: [linux-sunxi][PATCH] ARM: dts: sun4i: Enable HDMI support on the MK802
On Tue, Jan 30, 2018 at 07:32:54PM +0100, codekip...@gmail.com wrote: > From: Marcus Cooper > > Enable the display pipeline and HDMI output. > > Signed-off-by: Marcus Cooper Please put Chen-Yu in Cc of your patches. Applied, thanks Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout. signature.asc Description: PGP signature
Re: [linux-sunxi] Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.
On Wed, Jan 31, 2018 at 08:42:12AM +0100, Maxime Ripard wrote: > Hi Liviu, Hi Maxime, > > On Wed, Jan 31, 2018 at 03:08:08AM +, Liviu Dudau wrote: > > On Fri, Jan 26, 2018 at 11:00:41AM +0800, Yong wrote: > > > Hi Maxime, > > > > > > On Fri, 26 Jan 2018 09:46:58 +0800 > > > Yong wrote: > > > > > > > Hi Maxime, > > > > > > > > Do you have any experience in solving this problem? > > > > It seems the PHYS_OFFSET maybe undeclared when the ARCH is not arm. > > > > > > Got it. > > > Should I add 'depends on ARM' in Kconfig? > > > > No, I don't think you should do that, you should fix the code. > > > > The dma_addr_t addr that you've got is ideally coming from > > dma_alloc_coherent(), > > in which case the addr is already "suitable" for use by the device (because > > the > > bus where the device is attached to does all the address translations). > > Like we're discussing in that other part of the thread with Thierry > and Arnd, things are slightly more complicated than that :) Yeah, sorry, my threading of the discussion was broken and I've seen the rest of the thread after I have replied. My bad! > > In our case, the bus where the device is attached will not do the > address translations, and shouldn't. In my view, the bus is already doing address translation at physical level, AFAIU it remaps the memory to zero. What you (we?) need is a simple bus driver that registers the correct virt_to_bus()/bus_to_virt() hooks for the device that do this translation at the DMA API level as well. > > > If you apply PHYS_OFFSET forcefully to it you might get unexpected > > results. > > Out of curiosity, what would be these unexpected results? If in the future (or a parallel world setup) the device is sitting behind an IOMMU, the addr value might well be smaller than PHYS_OFFSET and you will under-wrap, possibly starting to hit kernel physical addresses (or anything sitting at the top of the physical memory). >From my time playing with IOMMUs and PCI domains, I've learned to treat the >dma_addr_t as a cookie value and never try to do arithmetics with it. Best regards, Liviu > > Thanks! > Maxime > > -- > Maxime Ripard, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com > > -- > You received this message because you are subscribed to the Google Groups > "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to linux-sunxi+unsubscr...@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[linux-sunxi] Re: [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings
Hi, On 31/01/18 08:21, Maxime Ripard wrote: > Hi, > > On Mon, Jan 29, 2018 at 10:38:25AM +, Andre Przywara wrote: >> On 29/01/18 09:58, Maxime Ripard wrote: >>> On Mon, Jan 29, 2018 at 09:44:44AM +, Andre Przywara wrote: On 29/01/18 08:51, Maxime Ripard wrote: > On Mon, Jan 29, 2018 at 01:15:19AM +, Andre Przywara wrote: >> The existing sun8i-emac driver in U-Boot uses some preliminary bindings, >> which matched our own DTs. Now that the Linux kernel got a driver, lets >> update our probe code to handle those Linux DTs as well. >> The first patch adds the missing compatible strings for the pinctrl >> drivers, >> which is needed for using the sunxi_name_to_gpio() lookup function. >> Patch 2/3 updates the pinctrl parser used in the sun8i-emac driver, to >> cope >> with the new, generic Allwinner pinctrl bindings. >> The final patch extends the probe routine in the Ethernet driver to deal >> with both the old and the new bindings. > > Thanks for posting this > >> This series allows to copy in the DTs from the latest kernel. >> Unfortunately >> right now updating the DTs for the H5 and A64 breaks the build, as the >> resulting binary (which embeds the DT) gets to large and triggers our new >> image size check. > > Sigh... > >> As the H5 and H3 share most of the DT, we can't just update the H3 >> DTs either. Hopefully we find some neat trick to work around that. > > Is it just because of the DT size, or because there's more code? My impression the code itself is always growing a tiny bit over the weeks, but this time around it's really the DT update. The current A64 .dtbs in U-Boot are around 8KB, mainline is at 13KB. Similar for the H5: going from 9.5KB to 14.5KB. Since you did a pretty good job already in identifying the code hogs, I couldn't find *easy* mitigations over the weekend. One possible fix is to remove the second .dtb in the Pine64 case, for which I sent a patch Friday night. >>> >>> Since the DT is fed to the C preprocessor, we could also put some >>> #ifdef 0 around the nodes that are never used by U-Boot (like the >>> clocks, timer, psci, dma, GIC, RTC, RSB, etc.) >> >> Well yes, U-Boot itself actually only requires a *tiny* .dtb (I think >> /aliases, /chosen, the reg of USB and Ethernet). But to be honest I >> don't want to go there. First it would be a constant churn to keep this >> up-to-date, > > I'm not too worried about the churn, it would be there only for the > time until we fully migrate to the FAT environment, so one-two release > now. And we're not syncing the DT very often these days (now that we > have support for the EMAC and USB that is all U-Boot cares about). > >> but more importantly for proper UEFI boot we just reuse U-Boot's >> .dtb to pass it on to the kernel. That is actually the purpose of >> this whole exercise. That already works today (at least for A64), >> but would benefit from some updates. >> >> So I would refrain from tinkering with U-Boot's .dtbs. > > That sucks :/ > >>> This should give us some room. >>> Another thing that stuck out is the sha256 checksum. It's "default y" if you have FIT. We need FIT for the SPL loader - but we don't do or need the checksum there. Some people do FIT loading for the kernel and initrd in U-Boot proper, I suppose, but I am not sure how many depend on SHA256 checksums in their images. >>> >>> I think there was someone (Tom?) that said that it was useful in some >>> circumstances? >> >> Yes, I clearly see that it is *useful*, but I wonder how many people >> would actually miss it today? We would bring it back once we dumped >> ENV_IS_IN_MMC, so it's only temporarily. > > His words were stronger actually, and he said that we want to keep it. > >> I think we can just disable it in some defconfigs, to avoid collateral >> damage to other boards. >> If people have a special need, they can always disable the MMC env and >> enable stuff at their likings, it's just the standard "make >> .._defconfig; make" process that needs to be fixed with some band-aids >> for now. > > I really don't want to go down the "let's fix each defconfig when we > find out it broke", this is very likely to be broken with no-one > noticing. Yeah, that's probably true. Looks we are really running out of silver bullets :-( > Is this issue happening when you sync the whole DT, and would it break > if you just convert the EMAC binding? > > Otherwise, we might try to revive the DTC garbage collection of unused > nodes patches. This would prevent us from using the overlays on such a > DT, but that doesn't like like an unfair tradeoff. Well, what's the point in updating the DT if we then deviate from it again? I would rather suggest to postpone the DT update, until we ditch the MMC environment. The whole reason I wanted the update is to pass it on to Linu
Re: [linux-sunxi][PATCH] ARM: dts: sunxi: h3/h5: Add DAI node for HDMI
On 31 January 2018 at 08:16, maxime ripard wrote: > On Mon, Jan 29, 2018 at 11:35:27AM +0100, Jernej Škrabec wrote: >> Hi Maxime, >> >> (previously I respond only to linux-sunxi mailing list) >> >> >On Mon, Jan 29, 2018 at 10:22:23AM +0100, codekip...@gmail.com wrote: >> >> From: Marcus Cooper >> >> >> >> Add the new DAI block for I2S2 which is used for HDMI audio. >> >> >> >> Signed-off-by: Marcus Cooper >> > >> >queued for 4.17, thanks! >> >Maxime >> >> Please note that HDMI I2S has usable 4 I2S lanes, since HDMI >> supports 8 channel audio. As Marcus said, other blocks probably >> support them too, they are just not wired out on pins. > > I've dropped those patches for now. > >> Should we change compatible for HDMI? > > I guess, another way of doing things if they are strictly identical > but for the number of lanes they support would be to add a DT property > for that number of lanes. > That's fine...I'll look into adding a dt property and how we would map channels to lanes. Do you know of any examples?, BR, CK > Maxime > > -- > Maxime Ripard, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[linux-sunxi] Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.
On Wed, Jan 31, 2018 at 8:29 AM, Maxime Ripard wrote: > Hi Thierry, > > On Tue, Jan 30, 2018 at 11:01:50AM +0100, Thierry Reding wrote: >> On Tue, Jan 30, 2018 at 10:59:16AM +0100, Thierry Reding wrote: >> > On Tue, Jan 30, 2018 at 10:24:48AM +0100, Arnd Bergmann wrote: >> > > On Tue, Jan 30, 2018 at 8:54 AM, Maxime Ripard >> > > wrote: >> > > > On Mon, Jan 29, 2018 at 03:34:02PM +0100, Arnd Bergmann wrote: >> > > >> On Mon, Jan 29, 2018 at 10:25 AM, Linus Walleij >> > > >> wrote: >> > > >> > On Mon, Jan 29, 2018 at 9:25 AM, Maxime Ripard >> > > >> > wrote: >> > > >> >> On Sat, Jan 27, 2018 at 05:14:26PM +0100, Linus Walleij wrote: >> > > >> > > >> >> > > >> At one point we had discussed adding a 'dma-masters' property that >> > > >> lists all the buses on which a device can be a dma master, and >> > > >> the respective properties of those masters (iommu, coherency, >> > > >> offset, ...). >> > > >> >> > > >> IIRC at the time we decided that we could live without that >> > > >> complexity, >> > > >> but perhaps we cannot. >> > > > >> > > > Are you talking about this ? >> > > > https://elixir.free-electrons.com/linux/latest/source/Documentation/devicetree/bindings/dma/dma.txt#L41 >> > > > >> > > > It doesn't seem to be related to that issue to me. And in our >> > > > particular cases, all the devices are DMA masters, the RAM is just >> > > > mapped to another address. >> > > >> > > No, that's not the one I was thinking of. The idea at the time was much >> > > more generic, and not limited to dma engines. I don't recall the details, >> > > but I think that Thierry was either involved or made the proposal at the >> > > time. >> > >> > Yeah, I vaguely remember discussing something like this before. A quick >> > search through my inbox yielded these two threads, mostly related to >> > IOMMU but I think there were some mentions about dma-ranges and so on as >> > well. I'll have to dig deeper into those threads to refresh my memories, >> > but I won't get around to it until later today. >> > >> > If someone wants to read up on this in the meantime, here are the links: >> > >> > https://lkml.org/lkml/2014/4/27/346 >> > >> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/257200.html >> > >> > From a quick glance the issue of dma-ranges was something that we hand- >> > waved at the time. >> >> Also found this, which seems to be relevant as well: >> >> >> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/252715.html >> >> Adding Dave. > > Thanks for the pointers, I started to read through it. > > I guess we have to come up with two solutions here: a short term one > to address the users we already have in the kernel properly, and a > long term one where we could use that discussion as a starting point. > > For the short term one, could we just set the device dma_pfn_offset to > PHYS_OFFSET at probe time, and use our dma_addr_t directly later on, > or would this also cause some issues? That would certainly be an improvement over the current version, it keeps the hack more localized. That's fine with me. Note that both PHYS_OFFSET and dma_pfn_offset have architecture specific meanings and they could in theory change, so ideally we'd do that fixup somewhere in arch/arm/mach-sunxi/ at boot time before the driver gets probed, but this wouldn't work on arm64 if we need it there too. > For the long term plan, from what I read from the discussion, it's > mostly centered around IOMMU indeed, and we don't have that. What we > would actually need is to break the assumption that the DMA "parent" > bus is the DT node's parent bus. > > And I guess this could be done quite easily by adding a dma-parent > property with a phandle to the bus controller, that would have a > dma-ranges property of its own with the proper mapping described > there. It should be simple enough to support, but is there anything > that could prevent something like that to work properly (such as > preventing further IOMMU-related developments that were described in > those mail threads). I've thought about it a little bit now. A dma-parent property would nicely solve two problems: - a device on a memory mapped control bus that is a bus master on a different bus. This is the case we are talking about here AFAICT - a device that is on a different kind of bus (i2c, spi, usb, ...) but also happens to be a dma master on another bus. I suspect we have some of these today and they work by accident because we set the dma_mask and dma_map_ops quite liberally in the DT probe code, but it really shouldn't work according to our bindings. We may also have drivers that work around the issue by forcing the correct dma mask and map_ops today, which makes them work but is rather fragile. I can think of a couple of other problems that may or may not be relevant in the future that would require a more complex solution: - a device that is a bus master on more than one bus, e.g. a DMA engine that can copy
Re: [linux-sunxi] Re: [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings
Hi Maxime, On Wed, Jan 31, 2018 at 7:36 PM, Maxime Ripard wrote: > Hi Julian, > > On Wed, Jan 31, 2018 at 07:29:13PM +1100, Julian Calaby wrote: >> Hi Maxime, >> >> On Wed, Jan 31, 2018 at 7:21 PM, Maxime Ripard >> wrote: >> > Hi, >> > >> > On Mon, Jan 29, 2018 at 10:38:25AM +, Andre Przywara wrote: >> >> On 29/01/18 09:58, Maxime Ripard wrote: >> >> > On Mon, Jan 29, 2018 at 09:44:44AM +, Andre Przywara wrote: >> >> >> On 29/01/18 08:51, Maxime Ripard wrote: >> >> >>> On Mon, Jan 29, 2018 at 01:15:19AM +, Andre Przywara wrote: >> >> The existing sun8i-emac driver in U-Boot uses some preliminary >> >> bindings, >> >> which matched our own DTs. Now that the Linux kernel got a driver, >> >> lets >> >> update our probe code to handle those Linux DTs as well. >> >> The first patch adds the missing compatible strings for the pinctrl >> >> drivers, >> >> which is needed for using the sunxi_name_to_gpio() lookup function. >> >> Patch 2/3 updates the pinctrl parser used in the sun8i-emac driver, >> >> to cope >> >> with the new, generic Allwinner pinctrl bindings. >> >> The final patch extends the probe routine in the Ethernet driver to >> >> deal >> >> with both the old and the new bindings. >> >> >>> >> >> >>> Thanks for posting this >> >> >>> >> >> This series allows to copy in the DTs from the latest kernel. >> >> Unfortunately >> >> right now updating the DTs for the H5 and A64 breaks the build, as >> >> the >> >> resulting binary (which embeds the DT) gets to large and triggers >> >> our new >> >> image size check. >> >> >>> >> >> >>> Sigh... >> >> >>> >> >> As the H5 and H3 share most of the DT, we can't just update the H3 >> >> DTs either. Hopefully we find some neat trick to work around that. >> >> >>> >> >> >>> Is it just because of the DT size, or because there's more code? >> >> >> >> >> >> My impression the code itself is always growing a tiny bit over the >> >> >> weeks, but this time around it's really the DT update. >> >> >> The current A64 .dtbs in U-Boot are around 8KB, mainline is at 13KB. >> >> >> Similar for the H5: going from 9.5KB to 14.5KB. >> >> >> >> >> >> Since you did a pretty good job already in identifying the code hogs, I >> >> >> couldn't find *easy* mitigations over the weekend. >> >> >> One possible fix is to remove the second .dtb in the Pine64 case, for >> >> >> which I sent a patch Friday night. >> >> > >> >> > Since the DT is fed to the C preprocessor, we could also put some >> >> > #ifdef 0 around the nodes that are never used by U-Boot (like the >> >> > clocks, timer, psci, dma, GIC, RTC, RSB, etc.) >> >> >> >> Well yes, U-Boot itself actually only requires a *tiny* .dtb (I think >> >> /aliases, /chosen, the reg of USB and Ethernet). But to be honest I >> >> don't want to go there. First it would be a constant churn to keep this >> >> up-to-date, >> > >> > I'm not too worried about the churn, it would be there only for the >> > time until we fully migrate to the FAT environment, so one-two release >> > now. And we're not syncing the DT very often these days (now that we >> > have support for the EMAC and USB that is all U-Boot cares about). >> > >> >> but more importantly for proper UEFI boot we just reuse U-Boot's >> >> .dtb to pass it on to the kernel. That is actually the purpose of >> >> this whole exercise. That already works today (at least for A64), >> >> but would benefit from some updates. >> >> >> >> So I would refrain from tinkering with U-Boot's .dtbs. >> > >> > That sucks :/ >> > >> >> > This should give us some room. >> >> > >> >> >> Another thing that stuck out is the sha256 checksum. It's "default y" >> >> >> if >> >> >> you have FIT. We need FIT for the SPL loader - but we don't do or need >> >> >> the checksum there. >> >> >> Some people do FIT loading for the kernel and initrd in U-Boot proper, >> >> >> I >> >> >> suppose, but I am not sure how many depend on SHA256 checksums in their >> >> >> images. >> >> > >> >> > I think there was someone (Tom?) that said that it was useful in some >> >> > circumstances? >> >> >> >> Yes, I clearly see that it is *useful*, but I wonder how many people >> >> would actually miss it today? We would bring it back once we dumped >> >> ENV_IS_IN_MMC, so it's only temporarily. >> > >> > His words were stronger actually, and he said that we want to keep it. >> > >> >> I think we can just disable it in some defconfigs, to avoid collateral >> >> damage to other boards. >> >> If people have a special need, they can always disable the MMC env and >> >> enable stuff at their likings, it's just the standard "make >> >> .._defconfig; make" process that needs to be fixed with some band-aids >> >> for now. >> > >> > I really don't want to go down the "let's fix each defconfig when we >> > find out it broke", this is very likely to be broken with no-one >> > noticing. >> > >> > Is this issue happenin
Re: [linux-sunxi] Re: [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings
Hi Julian, On Wed, Jan 31, 2018 at 07:29:13PM +1100, Julian Calaby wrote: > Hi Maxime, > > On Wed, Jan 31, 2018 at 7:21 PM, Maxime Ripard > wrote: > > Hi, > > > > On Mon, Jan 29, 2018 at 10:38:25AM +, Andre Przywara wrote: > >> On 29/01/18 09:58, Maxime Ripard wrote: > >> > On Mon, Jan 29, 2018 at 09:44:44AM +, Andre Przywara wrote: > >> >> On 29/01/18 08:51, Maxime Ripard wrote: > >> >>> On Mon, Jan 29, 2018 at 01:15:19AM +, Andre Przywara wrote: > >> The existing sun8i-emac driver in U-Boot uses some preliminary > >> bindings, > >> which matched our own DTs. Now that the Linux kernel got a driver, > >> lets > >> update our probe code to handle those Linux DTs as well. > >> The first patch adds the missing compatible strings for the pinctrl > >> drivers, > >> which is needed for using the sunxi_name_to_gpio() lookup function. > >> Patch 2/3 updates the pinctrl parser used in the sun8i-emac driver, > >> to cope > >> with the new, generic Allwinner pinctrl bindings. > >> The final patch extends the probe routine in the Ethernet driver to > >> deal > >> with both the old and the new bindings. > >> >>> > >> >>> Thanks for posting this > >> >>> > >> This series allows to copy in the DTs from the latest kernel. > >> Unfortunately > >> right now updating the DTs for the H5 and A64 breaks the build, as the > >> resulting binary (which embeds the DT) gets to large and triggers our > >> new > >> image size check. > >> >>> > >> >>> Sigh... > >> >>> > >> As the H5 and H3 share most of the DT, we can't just update the H3 > >> DTs either. Hopefully we find some neat trick to work around that. > >> >>> > >> >>> Is it just because of the DT size, or because there's more code? > >> >> > >> >> My impression the code itself is always growing a tiny bit over the > >> >> weeks, but this time around it's really the DT update. > >> >> The current A64 .dtbs in U-Boot are around 8KB, mainline is at 13KB. > >> >> Similar for the H5: going from 9.5KB to 14.5KB. > >> >> > >> >> Since you did a pretty good job already in identifying the code hogs, I > >> >> couldn't find *easy* mitigations over the weekend. > >> >> One possible fix is to remove the second .dtb in the Pine64 case, for > >> >> which I sent a patch Friday night. > >> > > >> > Since the DT is fed to the C preprocessor, we could also put some > >> > #ifdef 0 around the nodes that are never used by U-Boot (like the > >> > clocks, timer, psci, dma, GIC, RTC, RSB, etc.) > >> > >> Well yes, U-Boot itself actually only requires a *tiny* .dtb (I think > >> /aliases, /chosen, the reg of USB and Ethernet). But to be honest I > >> don't want to go there. First it would be a constant churn to keep this > >> up-to-date, > > > > I'm not too worried about the churn, it would be there only for the > > time until we fully migrate to the FAT environment, so one-two release > > now. And we're not syncing the DT very often these days (now that we > > have support for the EMAC and USB that is all U-Boot cares about). > > > >> but more importantly for proper UEFI boot we just reuse U-Boot's > >> .dtb to pass it on to the kernel. That is actually the purpose of > >> this whole exercise. That already works today (at least for A64), > >> but would benefit from some updates. > >> > >> So I would refrain from tinkering with U-Boot's .dtbs. > > > > That sucks :/ > > > >> > This should give us some room. > >> > > >> >> Another thing that stuck out is the sha256 checksum. It's "default y" if > >> >> you have FIT. We need FIT for the SPL loader - but we don't do or need > >> >> the checksum there. > >> >> Some people do FIT loading for the kernel and initrd in U-Boot proper, I > >> >> suppose, but I am not sure how many depend on SHA256 checksums in their > >> >> images. > >> > > >> > I think there was someone (Tom?) that said that it was useful in some > >> > circumstances? > >> > >> Yes, I clearly see that it is *useful*, but I wonder how many people > >> would actually miss it today? We would bring it back once we dumped > >> ENV_IS_IN_MMC, so it's only temporarily. > > > > His words were stronger actually, and he said that we want to keep it. > > > >> I think we can just disable it in some defconfigs, to avoid collateral > >> damage to other boards. > >> If people have a special need, they can always disable the MMC env and > >> enable stuff at their likings, it's just the standard "make > >> .._defconfig; make" process that needs to be fixed with some band-aids > >> for now. > > > > I really don't want to go down the "let's fix each defconfig when we > > find out it broke", this is very likely to be broken with no-one > > noticing. > > > > Is this issue happening when you sync the whole DT, and would it break > > if you just convert the EMAC binding? > > > > Otherwise, we might try to revive the DTC garbage collection of unused > > nodes patches. This would prevent
Re: [linux-sunxi] Re: [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings
Hi Maxime, On Wed, Jan 31, 2018 at 7:21 PM, Maxime Ripard wrote: > Hi, > > On Mon, Jan 29, 2018 at 10:38:25AM +, Andre Przywara wrote: >> On 29/01/18 09:58, Maxime Ripard wrote: >> > On Mon, Jan 29, 2018 at 09:44:44AM +, Andre Przywara wrote: >> >> On 29/01/18 08:51, Maxime Ripard wrote: >> >>> On Mon, Jan 29, 2018 at 01:15:19AM +, Andre Przywara wrote: >> The existing sun8i-emac driver in U-Boot uses some preliminary bindings, >> which matched our own DTs. Now that the Linux kernel got a driver, lets >> update our probe code to handle those Linux DTs as well. >> The first patch adds the missing compatible strings for the pinctrl >> drivers, >> which is needed for using the sunxi_name_to_gpio() lookup function. >> Patch 2/3 updates the pinctrl parser used in the sun8i-emac driver, to >> cope >> with the new, generic Allwinner pinctrl bindings. >> The final patch extends the probe routine in the Ethernet driver to deal >> with both the old and the new bindings. >> >>> >> >>> Thanks for posting this >> >>> >> This series allows to copy in the DTs from the latest kernel. >> Unfortunately >> right now updating the DTs for the H5 and A64 breaks the build, as the >> resulting binary (which embeds the DT) gets to large and triggers our >> new >> image size check. >> >>> >> >>> Sigh... >> >>> >> As the H5 and H3 share most of the DT, we can't just update the H3 >> DTs either. Hopefully we find some neat trick to work around that. >> >>> >> >>> Is it just because of the DT size, or because there's more code? >> >> >> >> My impression the code itself is always growing a tiny bit over the >> >> weeks, but this time around it's really the DT update. >> >> The current A64 .dtbs in U-Boot are around 8KB, mainline is at 13KB. >> >> Similar for the H5: going from 9.5KB to 14.5KB. >> >> >> >> Since you did a pretty good job already in identifying the code hogs, I >> >> couldn't find *easy* mitigations over the weekend. >> >> One possible fix is to remove the second .dtb in the Pine64 case, for >> >> which I sent a patch Friday night. >> > >> > Since the DT is fed to the C preprocessor, we could also put some >> > #ifdef 0 around the nodes that are never used by U-Boot (like the >> > clocks, timer, psci, dma, GIC, RTC, RSB, etc.) >> >> Well yes, U-Boot itself actually only requires a *tiny* .dtb (I think >> /aliases, /chosen, the reg of USB and Ethernet). But to be honest I >> don't want to go there. First it would be a constant churn to keep this >> up-to-date, > > I'm not too worried about the churn, it would be there only for the > time until we fully migrate to the FAT environment, so one-two release > now. And we're not syncing the DT very often these days (now that we > have support for the EMAC and USB that is all U-Boot cares about). > >> but more importantly for proper UEFI boot we just reuse U-Boot's >> .dtb to pass it on to the kernel. That is actually the purpose of >> this whole exercise. That already works today (at least for A64), >> but would benefit from some updates. >> >> So I would refrain from tinkering with U-Boot's .dtbs. > > That sucks :/ > >> > This should give us some room. >> > >> >> Another thing that stuck out is the sha256 checksum. It's "default y" if >> >> you have FIT. We need FIT for the SPL loader - but we don't do or need >> >> the checksum there. >> >> Some people do FIT loading for the kernel and initrd in U-Boot proper, I >> >> suppose, but I am not sure how many depend on SHA256 checksums in their >> >> images. >> > >> > I think there was someone (Tom?) that said that it was useful in some >> > circumstances? >> >> Yes, I clearly see that it is *useful*, but I wonder how many people >> would actually miss it today? We would bring it back once we dumped >> ENV_IS_IN_MMC, so it's only temporarily. > > His words were stronger actually, and he said that we want to keep it. > >> I think we can just disable it in some defconfigs, to avoid collateral >> damage to other boards. >> If people have a special need, they can always disable the MMC env and >> enable stuff at their likings, it's just the standard "make >> .._defconfig; make" process that needs to be fixed with some band-aids >> for now. > > I really don't want to go down the "let's fix each defconfig when we > find out it broke", this is very likely to be broken with no-one > noticing. > > Is this issue happening when you sync the whole DT, and would it break > if you just convert the EMAC binding? > > Otherwise, we might try to revive the DTC garbage collection of unused > nodes patches. This would prevent us from using the overlays on such a > DT, but that doesn't like like an unfair tradeoff. Stupid question: As I understand it, the boot process is SPL => Full U-Boot => Linux. Would it therefore be possible to use a cut-down DT for the SPL (just the bits it cares about) then use a full one afterwards? I'm guessing
[linux-sunxi] Re: [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings
Hi, On Mon, Jan 29, 2018 at 10:38:25AM +, Andre Przywara wrote: > On 29/01/18 09:58, Maxime Ripard wrote: > > On Mon, Jan 29, 2018 at 09:44:44AM +, Andre Przywara wrote: > >> On 29/01/18 08:51, Maxime Ripard wrote: > >>> On Mon, Jan 29, 2018 at 01:15:19AM +, Andre Przywara wrote: > The existing sun8i-emac driver in U-Boot uses some preliminary bindings, > which matched our own DTs. Now that the Linux kernel got a driver, lets > update our probe code to handle those Linux DTs as well. > The first patch adds the missing compatible strings for the pinctrl > drivers, > which is needed for using the sunxi_name_to_gpio() lookup function. > Patch 2/3 updates the pinctrl parser used in the sun8i-emac driver, to > cope > with the new, generic Allwinner pinctrl bindings. > The final patch extends the probe routine in the Ethernet driver to deal > with both the old and the new bindings. > >>> > >>> Thanks for posting this > >>> > This series allows to copy in the DTs from the latest kernel. > Unfortunately > right now updating the DTs for the H5 and A64 breaks the build, as the > resulting binary (which embeds the DT) gets to large and triggers our new > image size check. > >>> > >>> Sigh... > >>> > As the H5 and H3 share most of the DT, we can't just update the H3 > DTs either. Hopefully we find some neat trick to work around that. > >>> > >>> Is it just because of the DT size, or because there's more code? > >> > >> My impression the code itself is always growing a tiny bit over the > >> weeks, but this time around it's really the DT update. > >> The current A64 .dtbs in U-Boot are around 8KB, mainline is at 13KB. > >> Similar for the H5: going from 9.5KB to 14.5KB. > >> > >> Since you did a pretty good job already in identifying the code hogs, I > >> couldn't find *easy* mitigations over the weekend. > >> One possible fix is to remove the second .dtb in the Pine64 case, for > >> which I sent a patch Friday night. > > > > Since the DT is fed to the C preprocessor, we could also put some > > #ifdef 0 around the nodes that are never used by U-Boot (like the > > clocks, timer, psci, dma, GIC, RTC, RSB, etc.) > > Well yes, U-Boot itself actually only requires a *tiny* .dtb (I think > /aliases, /chosen, the reg of USB and Ethernet). But to be honest I > don't want to go there. First it would be a constant churn to keep this > up-to-date, I'm not too worried about the churn, it would be there only for the time until we fully migrate to the FAT environment, so one-two release now. And we're not syncing the DT very often these days (now that we have support for the EMAC and USB that is all U-Boot cares about). > but more importantly for proper UEFI boot we just reuse U-Boot's > .dtb to pass it on to the kernel. That is actually the purpose of > this whole exercise. That already works today (at least for A64), > but would benefit from some updates. > > So I would refrain from tinkering with U-Boot's .dtbs. That sucks :/ > > This should give us some room. > > > >> Another thing that stuck out is the sha256 checksum. It's "default y" if > >> you have FIT. We need FIT for the SPL loader - but we don't do or need > >> the checksum there. > >> Some people do FIT loading for the kernel and initrd in U-Boot proper, I > >> suppose, but I am not sure how many depend on SHA256 checksums in their > >> images. > > > > I think there was someone (Tom?) that said that it was useful in some > > circumstances? > > Yes, I clearly see that it is *useful*, but I wonder how many people > would actually miss it today? We would bring it back once we dumped > ENV_IS_IN_MMC, so it's only temporarily. His words were stronger actually, and he said that we want to keep it. > I think we can just disable it in some defconfigs, to avoid collateral > damage to other boards. > If people have a special need, they can always disable the MMC env and > enable stuff at their likings, it's just the standard "make > .._defconfig; make" process that needs to be fixed with some band-aids > for now. I really don't want to go down the "let's fix each defconfig when we find out it broke", this is very likely to be broken with no-one noticing. Is this issue happening when you sync the whole DT, and would it break if you just convert the EMAC binding? Otherwise, we might try to revive the DTC garbage collection of unused nodes patches. This would prevent us from using the overlays on such a DT, but that doesn't like like an unfair tradeoff. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout. signature.asc Descript