Re: [linux-sunxi][PATCH] ARM: dts: sun7i: Enable HDMI support on the Olimex EVB

[Code Kipper]

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 = <_out_con>;
>> + };
>> + };
>> + };
>> +
>>   leds {
>>   compatible = "gpio-leds";
>>   pinctrl-names = "default";
>> @@ -79,6 +90,10 @@
>>   status = "okay";
>>  };
>>
>> + {
>> + status = "okay";
>> +};
>> +
>>   {
>>   status = "okay";
>>  };
>> @@ -107,6 +122,16 @@
>>   };
>>  };
>>
>> + {
>> + status = "okay";
>> +};
>> +
>> +_out {
>> + hdmi_out_con: endpoint {
>> + remote-endpoint = <_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

[kbuild test robot]

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(>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(>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(>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, _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(>dev, base,
   623   
_gpadc_regmap_config);
   624  if (IS_ERR(info->regmap)) {
   625  ret = PTR_ERR(info->regmap);
   626  dev_err(>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(>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_clk_get(>dev, "bus");
   644  

[linux-sunxi] Re: [PATCH v2 08/16] iio: adc: sun4i-gpadc-iio: rework: add interrupt support

[kbuild test robot]

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

[Quentin Schulz]

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,
> + 

[linux-sunxi] Re: [PATCH v2 08/16] iio: adc: sun4i-gpadc-iio: rework: add interrupt support

[Quentin Schulz]

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(>dev, "failed to get IRQ: %d\n", irq);
> + return irq;
> + }
> +
> + ret = devm_request_threaded_irq(>dev, irq, NULL,
> + sunxi_irq_thread, IRQF_ONESHOT,
> + dev_name(>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(>dev,
> -  SUN4I_GPADC_AUTOSUSPEND_DELAY);
> - pm_runtime_use_autosuspend(>dev);
> - pm_runtime_set_suspended(>dev);
> - pm_runtime_enable(>dev);
> + if (!info->data->support_irq) {
> + pm_runtime_set_autosuspend_delay(>dev,
> +  SUN4I_GPADC_AUTOSUSPEND_DELAY);
> + pm_runtime_use_autosuspend(>dev);
> + pm_runtime_set_suspended(>dev);
> + pm_runtime_enable(>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/d/optout.

[linux-sunxi] Re: [PATCH v2 06/16] iio: adc: sun4i-gpadc-iio: rework: support multiple sensors

[Quentin Schulz]

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 = _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 = _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(>dev);
>  
>   if (IS_ENABLED(CONFIG_THERMAL_OF)) {
> - info->tzd = thermal_zone_of_sensor_register(info->sensor_device,
> - 0, info,
> - _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, _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

[Philipp Rossak]

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

[Philipp Rossak]

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 = < CLK_BUS_THS>, < CLK_THS>;
+   clock-names = "bus", "mod";
+   resets = < 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

[Quentin Schulz]

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

[Quentin Schulz]

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 = < CLK_BUS_THS>, < CLK_THS>;
> + clock-names = "bus", "mod";
> + resets = < 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

[Code Kipper]

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 = <_out_con>;
>> + };
>> + };
>> + };
>> +
>>   leds {
>>   compatible = "gpio-leds";
>>   pinctrl-names = "default";
>> @@ -93,6 +104,10 @@
>>   status = "okay";
>>  };
>>
>> + {
>> + status = "okay";
>> +};
>> +
>>   {
>>   status = "okay";
>>  };
>> @@ -101,6 +116,16 @@
>>   status = "okay";
>>  };
>>
>> + {
>> + status = "okay";
>> +};
>> +
>> +_out {
>> + hdmi_out_con: endpoint {
>> + remote-endpoint = <_con_in>;
>> + };
>> +};
>> +
>>   {
>
> 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

[maxime ripard]

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

[Maxime Ripard]

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 = <_out_con>;
> + };
> + };
> + };
> +
>   leds {
>   compatible = "gpio-leds";
>   pinctrl-names = "default";
> @@ -79,6 +90,10 @@
>   status = "okay";
>  };
>  
> + {
> + status = "okay";
> +};
> +
>   {
>   status = "okay";
>  };
> @@ -107,6 +122,16 @@
>   };
>  };
>  
> + {
> + status = "okay";
> +};
> +
> +_out {
> + hdmi_out_con: endpoint {
> + remote-endpoint = <_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

[Maxime Ripard]

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

[Maxime Ripard]

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 = <_out_con>;
> + };
> + };
> + };
> +
>   leds {
>   compatible = "gpio-leds";
>   pinctrl-names = "default";
> @@ -93,6 +104,10 @@
>   status = "okay";
>  };
>  
> + {
> + status = "okay";
> +};
> +
>   {
>   status = "okay";
>  };
> @@ -101,6 +116,16 @@
>   status = "okay";
>  };
>  
> + {
> + status = "okay";
> +};
> +
> +_out {
> + hdmi_out_con: endpoint {
> + remote-endpoint = <_con_in>;
> + };
> +};
> +
>   {

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

[Maxime Ripard]

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.

[Liviu Dudau]

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

[Andre Przywara]

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 

Re: [linux-sunxi][PATCH] ARM: dts: sunxi: h3/h5: Add DAI node for HDMI

[Code Kipper]

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.

[Arnd Bergmann]

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 

Re: [linux-sunxi] Re: [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings

[Julian Calaby]

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 

Re: [linux-sunxi] Re: [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings

[Maxime Ripard]

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
> > 

Re: [linux-sunxi] Re: [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings

[Julian Calaby]

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 

[linux-sunxi] Re: [PATCH 0/3] sunxi: sun8i-emac: Update DT bindings

[Maxime Ripard]

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