Re: [PATCH v2 3/4] ARM: dts: sun8i: Add the H3/H5 CSI controller

2018-11-22 Thread Chen-Yu Tsai 
On Thu, Nov 22, 2018 at 7:45 PM Jagan Teki  wrote:
>
> On Wed, Nov 14, 2018 at 8:29 PM Maxime Ripard  
> wrote:
> >
> > From: Mylène Josserand 
> >
> > The H3 and H5 features the same CSI controller that was initially found on
> > the A31.
> >
> > Add a DT node for it.
> >
> > Signed-off-by: Mylène Josserand 
> > Signed-off-by: Maxime Ripard 
> > ---
> >  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 22 ++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi 
> > b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> > index 4b1530ebe427..8779ee750bd8 100644
> > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> > @@ -393,6 +393,13 @@
> > interrupt-controller;
> > #interrupt-cells = <3>;
> >
> > +   csi_pins: csi {
> > +   pins = "PE0", "PE1", "PE2", "PE3", "PE4",
> > +  "PE5", "PE6", "PE7", "PE8", "PE9",
> > +  "PE10", "PE11";
> > +   function = "csi";
> > +   };
> > +
> > emac_rgmii_pins: emac0 {
> > pins = "PD0", "PD1", "PD2", "PD3", "PD4",
> >"PD5", "PD7", "PD8", "PD9", "PD10",
> > @@ -744,6 +751,21 @@
> > interrupts =  > IRQ_TYPE_LEVEL_HIGH)>;
> > };
> >
> > +   csi: camera@1cb {
> > +   compatible = "allwinner,sun8i-h3-csi",
> > +"allwinner,sun6i-a31-csi";
> > +   reg = <0x01cb 0x1000>;
> > +   interrupts = ;
> > +   clocks = < CLK_BUS_CSI>,
> > +< CLK_CSI_SCLK>,
> > +< CLK_DRAM_CSI>;
> > +   clock-names = "bus", "mod", "ram";
>
> Don't we need CLK_CSI_MCLK which can be pinout via PE1?

The CSI hardware block does not have any controls for MCLK.
It's simply routed from the CCU directly to the pin.

ChenYu

Re: [PATCH v2 1/4] dt-bindings: media: sun6i: Add A31 and H3 compatibles

2018-11-14 Thread Chen-Yu Tsai 
On Wed, Nov 14, 2018 at 10:59 PM Maxime Ripard
 wrote:
>
> The H3 has a slightly different CSI controller (no BT656, no CCI) which
> looks a lot like the original A31 controller. Add a compatible for the A31,
> and more specific compatible the for the H3 to be used in combination for
> the A31.
>
> Reviewed-by: Rob Herring 
> Signed-off-by: Maxime Ripard 

Reviewed-by: Chen-Yu Tsai

Re: [PATCH v2 2/4] media: sun6i: Add A31 compatible

2018-11-14 Thread Chen-Yu Tsai 
On Wed, Nov 14, 2018 at 10:59 PM Maxime Ripard
 wrote:
>
> The first device that used that IP was the A31. Add it to our list of
> compatibles.
>
> Signed-off-by: Maxime Ripard 

Reviewed-by: Chen-Yu Tsai

Re: [PATCH v2 3/4] ARM: dts: sun8i: Add the H3/H5 CSI controller

2018-11-14 Thread Chen-Yu Tsai 
Hi,

On Wed, Nov 14, 2018 at 10:59 PM Maxime Ripard
 wrote:
>
> From: Mylène Josserand 
>
> The H3 and H5 features the same CSI controller that was initially found on
> the A31.
>
> Add a DT node for it.
>
> Signed-off-by: Mylène Josserand 
> Signed-off-by: Maxime Ripard 
> ---
>  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 22 ++
>  1 file changed, 22 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi 
> b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> index 4b1530ebe427..8779ee750bd8 100644
> --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> @@ -393,6 +393,13 @@
> interrupt-controller;
> #interrupt-cells = <3>;
>
> +   csi_pins: csi {
> +   pins = "PE0", "PE1", "PE2", "PE3", "PE4",

You should separate out PE1, which provides the MCLK. Not all camera modules
need it. Some might have a standalone crystal to provide the reference clock.
Designs could then use that pin for other purposes.

Otherwise,

Reviewed-by: Chen-Yu Tsai 

> +  "PE5", "PE6", "PE7", "PE8", "PE9",
> +  "PE10", "PE11";
> +   function = "csi";
> +   };
> +
> emac_rgmii_pins: emac0 {
> pins = "PD0", "PD1", "PD2", "PD3", "PD4",
>"PD5", "PD7", "PD8", "PD9", "PD10",
> @@ -744,6 +751,21 @@
> interrupts =  IRQ_TYPE_LEVEL_HIGH)>;
> };
>
> +   csi: camera@1cb {
> +   compatible = "allwinner,sun8i-h3-csi",
> +"allwinner,sun6i-a31-csi";
> +   reg = <0x01cb 0x1000>;
> +   interrupts = ;
> +   clocks = < CLK_BUS_CSI>,
> +< CLK_CSI_SCLK>,
> +< CLK_DRAM_CSI>;
> +   clock-names = "bus", "mod", "ram";
> +   resets = < RST_BUS_CSI>;
> +   pinctrl-names = "default";
> +   pinctrl-0 = <_pins>;
> +   status = "disabled";
> +   };
> +
> hdmi: hdmi@1ee {
> compatible = "allwinner,sun8i-h3-dw-hdmi",
>  "allwinner,sun8i-a83t-dw-hdmi";
> --
> 2.19.1
>

Re: [PATCH v3 02/14] drivers: soc: sunxi: Add dedicated compatibles for the A13, A20 and A33

2018-05-10 Thread Chen-Yu Tsai 
On Mon, May 7, 2018 at 5:44 AM, Paul Kocialkowski
 wrote:
> This introduces platform-specific compatibles for the A13, A20 and A33
> SRAM driver. No particular adaptation for these platforms is required at
> this point, although this might become the case in the future.
>
> Signed-off-by: Paul Kocialkowski 
> ---
>  drivers/soc/sunxi/sunxi_sram.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/soc/sunxi/sunxi_sram.c b/drivers/soc/sunxi/sunxi_sram.c
> index 74cb81f37bd6..43ebc3bd33f2 100644
> --- a/drivers/soc/sunxi/sunxi_sram.c
> +++ b/drivers/soc/sunxi/sunxi_sram.c
> @@ -315,6 +315,9 @@ static int sunxi_sram_probe(struct platform_device *pdev)
>
>  static const struct of_device_id sunxi_sram_dt_match[] = {
> { .compatible = "allwinner,sun4i-a10-sram-controller" },
> +   { .compatible = "allwinner,sun5i-a13-sram-controller" },
> +   { .compatible = "allwinner,sun7i-a20-sram-controller" },
> +   { .compatible = "allwinner,sun8i-a33-sram-controller" },

We should probably name these "system-controller". Maxime?

ChenYu

> { .compatible = "allwinner,sun50i-a64-sram-controller" },
> { },
>  };
> --
> 2.16.3
>

Re: [linux-sunxi] Re: [PATCH v6 2/2] media: V3s: Add support for Allwinner CSI.

2018-01-30 Thread Chen-Yu Tsai 
On Wed, Jan 31, 2018 at 11:08 AM, 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). If you
> apply PHYS_OFFSET forcefully to it you might get unexpected results.

As explained in the thread, the dma_addr_t address is based on the kernel
and processor's viewpoint, which has DRAM at an offset. This particular
peripheral (and some others, such as display and video decoder) on Allwinner
platforms do DMA on the separate memory bus directly, which does _not_
have that memory offset. This is specific to our hardware. And also mentioned
is that there is no sensible representation in the device tree that would
allow the DMA API to do proper address translation.

Just throwing it out there, maybe we could do a dummy IOMMU that does the
simple translation of (addr - PHYS_OFFSET)? Still I'm not sure if the device
tree representation would be sane.

ChenYu

>>
>> >
>> > On Fri, 26 Jan 2018 08:04:18 +0800
>> > kbuild test robot  wrote:
>> >
>> > > Hi Yong,
>> > >
>> > > I love your patch! Yet something to improve:
>> > >
>> > > [auto build test ERROR on linuxtv-media/master]
>> > > [also build test ERROR on v4.15-rc9 next-20180119]
>> > > [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/Yong-Deng/dt-bindings-media-Add-Allwinner-V3s-Camera-Sensor-Interface-CSI/20180126-054511
>> > > base:   git://linuxtv.org/media_tree.git master
>> > > config: i386-allmodconfig (attached as .config)
>> > > compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
>> > > reproduce:
>> > > # save the attached .config to linux build tree
>> > > make ARCH=i386
>> > >
>> > > All errors (new ones prefixed by >>):
>> > >
>> > >drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c: In function 
>> > > 'sun6i_csi_update_buf_addr':
>> > > >> drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c:567:31: error: 
>> > > >> 'PHYS_OFFSET' undeclared (first use in this function); did you mean 
>> > > >> 'PAGE_OFFSET'?
>> > >  dma_addr_t bus_addr = addr - PHYS_OFFSET;
>> > >   ^~~
>> > >   PAGE_OFFSET
>> > >drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c:567:31: note: each 
>> > > undeclared identifier is reported only once for each function it appears 
>> > > in
>> > >
>> > > vim +567 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
>> > >
>> > >562
>> > >563void sun6i_csi_update_buf_addr(struct sun6i_csi *csi, 
>> > > dma_addr_t addr)
>> > >564{
>> > >565struct sun6i_csi_dev *sdev = sun6i_csi_to_dev(csi);
>> > >566/* transform physical address to bus address */
>> > >  > 567dma_addr_t bus_addr = addr - PHYS_OFFSET;
>> > >568
>> > >569regmap_write(sdev->regmap, CSI_CH_F0_BUFA_REG,
>> > >570 (bus_addr + sdev->planar_offset[0]) >> 2);
>> > >571if (sdev->planar_offset[1] != -1)
>> > >572regmap_write(sdev->regmap, CSI_CH_F1_BUFA_REG,
>> > >573 (bus_addr + 
>> > > sdev->planar_offset[1]) >> 2);
>> > >574if (sdev->planar_offset[2] != -1)
>> > >575regmap_write(sdev->regmap, CSI_CH_F2_BUFA_REG,
>> > >576 (bus_addr + 
>> > > sdev->planar_offset[2]) >> 2);
>> > >577}
>> > >578
>> > >
>> > > ---
>> > > 0-DAY kernel test infrastructureOpen Source Technology 
>> > > Center
>> > > https://lists.01.org/pipermail/kbuild-all   Intel 
>> > > Corporation
>> >
>> >
>> > Thanks,
>> > Yong
>>
>>
>> Thanks,
>> Yong
>>
>> --
>> 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 v3 1/3] media: V3s: Add support for Allwinner CSI.

2017-12-19 Thread Chen-Yu Tsai 
On Mon, Nov 13, 2017 at 3:30 PM, Yong Deng  wrote:
> Allwinner V3s SoC have two CSI module. CSI0 is used for MIPI interface
> and CSI1 is used for parallel interface. This is not documented in
> datasheet but by testing and guess.
>
> This patch implement a v4l2 framework driver for it.
>
> Currently, the driver only support the parallel interface. MIPI-CSI2,
> ISP's support are not included in this patch.
>
> Signed-off-by: Yong Deng 
> ---
>  drivers/media/platform/Kconfig |   1 +
>  drivers/media/platform/Makefile|   2 +
>  drivers/media/platform/sunxi/sun6i-csi/Kconfig |   9 +
>  drivers/media/platform/sunxi/sun6i-csi/Makefile|   3 +
>  drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c | 918 
> +
>  drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h | 146 
>  .../media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h | 203 +
>  .../media/platform/sunxi/sun6i-csi/sun6i_video.c   | 722 
>  .../media/platform/sunxi/sun6i-csi/sun6i_video.h   |  61 ++
>  9 files changed, 2065 insertions(+)
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/Kconfig
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/Makefile
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.h
>

[...]

> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c 
> b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
> new file mode 100644
> index 000..0cebcbd
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c

[...]

> +/* 
> -
> + * Media Operations
> + */
> +static int sun6i_video_formats_init(struct sun6i_video *video)
> +{
> +   struct v4l2_subdev_mbus_code_enum mbus_code = { 0 };
> +   struct sun6i_csi *csi = video->csi;
> +   struct v4l2_format format;
> +   struct v4l2_subdev *subdev;
> +   u32 pad;
> +   const u32 *pixformats;
> +   int pixformat_count = 0;
> +   u32 subdev_codes[32]; /* subdev format codes, 32 should be enough */
> +   int codes_count = 0;
> +   int num_fmts = 0;
> +   int i, j;
> +
> +   subdev = sun6i_video_remote_subdev(video, );
> +   if (subdev == NULL)
> +   return -ENXIO;
> +
> +   /* Get supported pixformats of CSI */
> +   pixformat_count = sun6i_csi_get_supported_pixformats(csi, 
> );
> +   if (pixformat_count <= 0)
> +   return -ENXIO;
> +
> +   /* Get subdev formats codes */
> +   mbus_code.pad = pad;
> +   mbus_code.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> +   while (!v4l2_subdev_call(subdev, pad, enum_mbus_code, NULL,
> +_code)) {
> +   if (codes_count >= ARRAY_SIZE(subdev_codes)) {
> +   dev_warn(video->csi->dev,
> +"subdev_codes array is full!\n");
> +   break;
> +   }
> +   subdev_codes[codes_count] = mbus_code.code;
> +   codes_count++;
> +   mbus_code.index++;
> +   }
> +
> +   if (!codes_count)
> +   return -ENXIO;
> +
> +   /* Get supported formats count */
> +   for (i = 0; i < codes_count; i++) {
> +   for (j = 0; j < pixformat_count; j++) {
> +   if (!sun6i_csi_is_format_support(csi, pixformats[j],
> +   mbus_code.code)) {

Bug here. You are testing against mbus_code.code, which would be whatever
was leftover from the previous section. Instead you should be testing
against subdev_codes[i], the list you just built.

Without it, it won't get past this part of the code if the last enumerated
media bus format isn't supported.

> +   continue;
> +   }
> +   num_fmts++;
> +   }
> +   }
> +
> +   if (!num_fmts)
> +   return -ENXIO;
> +
> +   video->num_formats = num_fmts;
> +   video->formats = devm_kcalloc(video->csi->dev, num_fmts,
> +   sizeof(struct sun6i_csi_format), GFP_KERNEL);
> +   if (!video->formats)
> +   return -ENOMEM;
> +
> +   /* Get supported formats */
> +   num_fmts = 0;
> +   for (i = 0; i < codes_count; i++) {
> +   for (j = 0; j < pixformat_count; j++) {
> +   if (!sun6i_csi_is_format_support(csi, pixformats[j],
> +   mbus_code.code)) {

Same here.

This gets me past the enumeration part of things...

> +

Re: [linux-sunxi] [PATCH v3 2/3] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI)

2017-12-18 Thread Chen-Yu Tsai 
On Mon, Dec 18, 2017 at 5:43 PM, Yong  wrote:
> Hi,
>
> On Mon, 18 Dec 2017 10:24:37 +0100
> Maxime Ripard  wrote:
>
>> Hi,
>>
>> On Mon, Dec 18, 2017 at 04:49:21PM +0800, Yong wrote:
>> > > > +   compatible = "allwinner,sun8i-v3s-csi";
>> > > > +   reg = <0x01cb4000 0x1000>;
>> > > > +   interrupts = ;
>> > > > +   clocks = < CLK_BUS_CSI>,
>> > > > +< CLK_CSI1_SCLK>,
>> > >
>> > > CSI also has an MCLK. Do you need that one?
>> >
>> > MCLK is not needed if the front end is not a sensor (like adv7611).
>> > I will add it as an option.
>>
>> I guess this should always be needed then. And the driver will make
>> the decision to enable it or not.
>
> But how the driver to know if it should be enabled?
> I think MCLK should be added in DT just if the subdev need it.

Looking around the docs, there doesn't seem to be anything related
to MCLK within the CSI section.

Furthermore, camera sensor bindings, such as for the OV5640, in fact
do have a property for a reference clock, which is called XCLK or
XVCLK.

Since the clock is already exported from the CCU, I suppose it's
just a matter of referencing the clock from the camera node, with
the proper pinctrl for that pin.

So to summarize, just ignore my comment about the missing MCLK. :)

ChenYu

Re: [linux-sunxi] [PATCH v3 2/3] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI)

2017-12-18 Thread Chen-Yu Tsai 
On Mon, Nov 13, 2017 at 3:32 PM, Yong Deng  wrote:
> Add binding documentation for Allwinner V3s CSI.
>
> Signed-off-by: Yong Deng 
> ---
>  .../devicetree/bindings/media/sun6i-csi.txt| 51 
> ++
>  1 file changed, 51 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt
>
> diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt 
> b/Documentation/devicetree/bindings/media/sun6i-csi.txt
> new file mode 100644
> index 000..f3916a2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt
> @@ -0,0 +1,51 @@
> +Allwinner V3s Camera Sensor Interface
> +--
> +
> +Required properties:
> +  - compatible: value must be "allwinner,sun8i-v3s-csi"
> +  - reg: base address and size of the memory-mapped region.
> +  - interrupts: interrupt associated to this IP
> +  - clocks: phandles to the clocks feeding the CSI
> +* bus: the CSI interface clock
> +* mod: the CSI module clock
> +* ram: the CSI DRAM clock
> +  - clock-names: the clock names mentioned above
> +  - resets: phandles to the reset line driving the CSI
> +
> +- ports: A ports node with endpoint definitions as defined in
> +  Documentation/devicetree/bindings/media/video-interfaces.txt.
> +  Currently, the driver only support the parallel interface. So, a single 
> port
> +  node with one endpoint and parallel bus is supported.
> +
> +Example:
> +
> +   csi1: csi@01cb4000 {

Drop the leading zero in the address part.

> +   compatible = "allwinner,sun8i-v3s-csi";
> +   reg = <0x01cb4000 0x1000>;
> +   interrupts = ;
> +   clocks = < CLK_BUS_CSI>,
> +< CLK_CSI1_SCLK>,

CSI also has an MCLK. Do you need that one?

ChenYu

> +< CLK_DRAM_CSI>;
> +   clock-names = "bus", "mod", "ram";
> +   resets = < RST_BUS_CSI>;
> +
> +   port {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   /* Parallel bus endpoint */
> +   csi1_ep: endpoint {
> +   remote-endpoint = <_ep>;
> +   bus-width = <16>;
> +   data-shift = <0>;
> +
> +   /* If hsync-active/vsync-active are missing,
> +  embedded BT.656 sync is used */
> +   hsync-active = <0>; /* Active low */
> +   vsync-active = <0>; /* Active low */
> +   data-active = <1>;  /* Active high */
> +   pclk-sample = <1>;  /* Rising */
> +   };
> +   };
> +   };
> +
> --
> 1.8.3.1
>
> --
> 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: [PATCH 5/5] arm: dts: sun8i: a83t: bananapi-m3: Enable IR controller

2017-12-17 Thread Chen-Yu Tsai 
On Mon, Dec 18, 2017 at 6:45 AM, Philipp Rossak <embe...@gmail.com> wrote:
> The Bananapi M3 has an onboard IR receiver.
> This enables the onboard IR receiver subnode.
> Other than the other IR receivers this one needs a base clock frequency

Unlike the other...

> of 300 Hz (3 MHz), to be able to work.
>
> Signed-off-by: Philipp Rossak <embe...@gmail.com>
> ---
>  arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts 
> b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
> index 6550bf0e594b..2bf25ca64133 100644
> --- a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
> +++ b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
> @@ -100,6 +100,13 @@
> status = "okay";
>  };
>
> + {

See my other reply about the name.

Otherwise,

Acked-by: Chen-Yu Tsai <w...@csie.org>

> +   pinctrl-names = "default";
> +   pinctrl-0 = <_pins_a>;
> +   clock-frequency = <300>;
> +   status = "okay";
> +};
> +
>   {
> rgmii_phy: ethernet-phy@1 {
> compatible = "ethernet-phy-ieee802.3-c22";
> --
> 2.11.0
>

Re: [PATCH 3/5] arm: dts: sun8i: a83t: Add the ir pin for the A83T

2017-12-17 Thread Chen-Yu Tsai 
On Mon, Dec 18, 2017 at 6:45 AM, Philipp Rossak  wrote:
> The CIR Pin of the A83T is located at PL12.
>
> Signed-off-by: Philipp Rossak 
> ---
>  arch/arm/boot/dts/sun8i-a83t.dtsi | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi 
> b/arch/arm/boot/dts/sun8i-a83t.dtsi
> index a384b766f3dc..954c2393325f 100644
> --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> @@ -521,6 +521,11 @@
> drive-strength = <20>;
> bias-pull-up;
> };
> +
> +   ir_pins_a: ir@0 {

ir_pins: ir-pins

And it really should be cir, to distinguish it from IrDA.

ChenYu

> +   pins = "PL12";
> +   function = "s_cir_rx";
> +   };
> };
>
> r_rsb: rsb@1f03400 {
> --
> 2.11.0
>

Re: [PATCH RFC 2/2] dt-bindings: add binding documentation for Allwinner CSI

2017-06-29 Thread Chen-Yu Tsai 
On Fri, Jun 30, 2017 at 5:19 AM, Rob Herring  wrote:
> On Tue, Jun 27, 2017 at 07:07:34PM +0800, Yong Deng wrote:
>> Add binding documentation for Allwinner CSI.
>
> For the subject:
>
> dt-bindings: media: Add Allwinner Camera Sensor Interface (CSI)
>
> "binding documentation" is redundant.
>
>>
>> Signed-off-by: Yong Deng 
>> ---
>>  .../devicetree/bindings/media/sunxi-csi.txt| 51 
>> ++
>>  1 file changed, 51 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/sunxi-csi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/sunxi-csi.txt 
>> b/Documentation/devicetree/bindings/media/sunxi-csi.txt
>> new file mode 100644
>> index 000..770be0e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/sunxi-csi.txt
>> @@ -0,0 +1,51 @@
>> +Allwinner V3s Camera Sensor Interface
>> +--
>> +
>> +Required properties:
>> +  - compatible: value must be "allwinner,sun8i-v3s-csi"
>> +  - reg: base address and size of the memory-mapped region.
>> +  - interrupts: interrupt associated to this IP
>> +  - clocks: phandles to the clocks feeding the CSI
>> +* ahb: the CSI interface clock
>> +* mod: the CSI module clock
>> +* ram: the CSI DRAM clock
>> +  - clock-names: the clock names mentioned above
>> +  - resets: phandles to the reset line driving the CSI
>> +
>> +- ports: A ports node with endpoint definitions as defined in
>> +  Documentation/devicetree/bindings/media/video-interfaces.txt. The
>> +  first port should be the input endpoints, the second one the outputs
>
> Is there more than one endpoint for each port? If so, need to define
> that numbering too.

It is possible to have multiple camera sensors connected to the same
bus. Think front and back cameras on a cell phone or tablet.

I don't think any kind of numbering makes much sense though. The
system is free to use just one sensor at a time, or use many with
some time multiplexing scheme. What might matter to the end user
is where the camera is placed. But using the position or orientation
as a numbering scheme might not work well either. Someone may end
up using two sensors with the same orientation for stereoscopic
vision.

>
>> +
>> +Example:
>> +
>> + csi1: csi@01cb4000 {
>> + compatible = "allwinner,sun8i-v3s-csi";
>> + reg = <0x01cb4000 0x1000>;

Yong, the address range size is 0x4000, including the CCI (I2C)
controller at offset 0x3000. You should also consider this in
the device tree binding, and the driver.

ChenYu

>> + interrupts = ;
>> + clocks = < CLK_BUS_CSI>,
>> +  < CLK_CSI1_SCLK>,
>> +  < CLK_DRAM_CSI>;
>> + clock-names = "ahb", "mod", "ram";
>> + resets = < RST_BUS_CSI>;
>> +
>> + port {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + /* Parallel bus endpoint */
>> + csi1_0: endpoint@0 {
>> + reg = <0>;
>
> Don't need this and everything associated with it for a single endpoint.
>
>> + remote = <_1>;
>> + bus-width = <16>;
>> + data-shift = <0>;
>> +
>> + /* If hsync-active/vsync-active are missing,
>> +embedded BT.656 sync is used */
>> + hsync-active = <0>; /* Active low */
>> + vsync-active = <0>; /* Active low */
>> + data-active = <1>;  /* Active high */
>> + pclk-sample = <1>;  /* Rising */
>> + };
>> + };
>> + };
>> +
>> --
>> 1.8.3.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] [media] rc: sunxi-cir: Initialize the spinlock properly

2015-12-21 Thread Chen-Yu Tsai 
The driver allocates the spinlock but fails to initialize it correctly.
The kernel reports a BUG indicating bad spinlock magic when spinlock
debugging is enabled.

Call spin_lock_init() on it to initialize it correctly.

Fixes: b4e3e59fb59c ("[media] rc: add sunxi-ir driver")
Signed-off-by: Chen-Yu Tsai <w...@csie.org>
---
 drivers/media/rc/sunxi-cir.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
index 7830aef3db45..40f77685cc4a 100644
--- a/drivers/media/rc/sunxi-cir.c
+++ b/drivers/media/rc/sunxi-cir.c
@@ -153,6 +153,8 @@ static int sunxi_ir_probe(struct platform_device *pdev)
if (!ir)
return -ENOMEM;
 
+   spin_lock_init(>ir_lock);
+
if (of_device_is_compatible(dn, "allwinner,sun5i-a13-ir"))
ir->fifo_size = 64;
else
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-sunxi] [PATCH v2 04/13] rc: sunxi-cir: Add support for an optional reset controller

2015-01-19 Thread Chen-Yu Tsai 
Hi,

On Sat, Dec 20, 2014 at 6:20 PM, Hans de Goede hdego...@redhat.com wrote:
 Hi,


 On 19-12-14 19:17, Maxime Ripard wrote:

 Hi,

 On Thu, Dec 18, 2014 at 09:50:26AM +0100, Hans de Goede wrote:

 Hi,

 On 18-12-14 03:48, Chen-Yu Tsai wrote:

 Hi,

 On Thu, Dec 18, 2014 at 1:18 AM, Hans de Goede hdego...@redhat.com
 wrote:

 On sun6i the cir block is attached to the reset controller, add support
 for de-asserting the reset if a reset controller is specified in dt.

 Signed-off-by: Hans de Goede hdego...@redhat.com
 Acked-by: Mauro Carvalho Chehab mche...@osg.samsung.com
 Acked-by: Maxime Ripard maxime.rip...@free-electrons.com
 ---
   .../devicetree/bindings/media/sunxi-ir.txt |  2 ++
   drivers/media/rc/sunxi-cir.c   | 25
 --
   2 files changed, 25 insertions(+), 2 deletions(-)

 diff --git a/Documentation/devicetree/bindings/media/sunxi-ir.txt
 b/Documentation/devicetree/bindings/media/sunxi-ir.txt
 index 23dd5ad..6b70b9b 100644
 --- a/Documentation/devicetree/bindings/media/sunxi-ir.txt
 +++ b/Documentation/devicetree/bindings/media/sunxi-ir.txt
 @@ -10,6 +10,7 @@ Required properties:

   Optional properties:
   - linux,rc-map-name : Remote control map name.
 +- resets : phandle + reset specifier pair


 Should it be optional? Or should we use a sun6i compatible with
 a mandatory reset phandle? I mean, the driver/hardware is not
 going to work with the reset missing on sun6i.

 Seems we are doing it one way for some of our drivers, and
 the other (optional) way for more generic ones, like USB.


 I do not believe that we should add a new compatible just because
 the reset line of a block is hooked up differently. It is the
 exact same ip-block. Only now the reset is not controlled
 through the apb-gate, but controlled separately.


 He has a point though. Your driver might very well probe nicely and
 everything, but still wouldn't be functional at all because the reset
 line wouldn't have been specified in the DT.


 Right, just like other drivers we've, see e.g.:

 Documentation/devicetree/bindings/mmc/sunxi-mmc.txt

 Which is dealing with this in the same way.

 The easiest way to deal with that would be in the bindings doc to
 update it with a compatible for the A31, and mentionning that the
 reset property is mandatory there.


 No the easiest way to deal with this is to expect people writing
 the dts to know what they are doing, just like we do for a lot
 of the other blocks in sun6i.

 Maybe put a generic note somewhere that sun6i has a reset controller,
 and that for all the blocks with optional resets property it should
 be considered mandatory on sun6i ?

 I'm sorry but I'm not going to make this change for the ir bindings
 given that we've the same situation in a lot of other places.

 Consistency is important. Moreover I believe that having a sun6i
 specific compatible string is just wrong, since it is the exact
 same hardware block as on sun5i, just with its reset line routed
 differently, just like e.g. the mmc controller, the uarts or the gmac
 all of which also do not have a sun6i specific compatible to enforce
 reset controller usage.

 Regards,

 Hans




 Note that the code itself might not change at all though. I'd just
 like to avoid any potential breaking of the DT bindings themselves. If
 we further want to refine the code, we can do that however we want.

 I have a slight preference for a clean error if reset is missing, but
 I won't get in the way just for that.

Seems this patch and the following patch were overlooked after the
discussion. Any chance we could get this in? Hans has made a good
argument, so I take back any doubts I raised.

ChenYu
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-sunxi] [PATCH v2 04/13] rc: sunxi-cir: Add support for an optional reset controller

2015-01-19 Thread Chen-Yu Tsai 
On Mon, Jan 19, 2015 at 10:17 PM, Hans de Goede hdego...@redhat.com wrote:
 Hi,


 On 19-01-15 15:10, Chen-Yu Tsai wrote:

 Hi,

 On Sat, Dec 20, 2014 at 6:20 PM, Hans de Goede hdego...@redhat.com
 wrote:

 Hi,


 On 19-12-14 19:17, Maxime Ripard wrote:


 Hi,

 On Thu, Dec 18, 2014 at 09:50:26AM +0100, Hans de Goede wrote:


 Hi,

 On 18-12-14 03:48, Chen-Yu Tsai wrote:


 Hi,

 On Thu, Dec 18, 2014 at 1:18 AM, Hans de Goede hdego...@redhat.com
 wrote:


 On sun6i the cir block is attached to the reset controller, add
 support
 for de-asserting the reset if a reset controller is specified in dt.

 Signed-off-by: Hans de Goede hdego...@redhat.com
 Acked-by: Mauro Carvalho Chehab mche...@osg.samsung.com
 Acked-by: Maxime Ripard maxime.rip...@free-electrons.com
 ---
.../devicetree/bindings/media/sunxi-ir.txt |  2 ++
drivers/media/rc/sunxi-cir.c   | 25
 --
2 files changed, 25 insertions(+), 2 deletions(-)

 diff --git a/Documentation/devicetree/bindings/media/sunxi-ir.txt
 b/Documentation/devicetree/bindings/media/sunxi-ir.txt
 index 23dd5ad..6b70b9b 100644
 --- a/Documentation/devicetree/bindings/media/sunxi-ir.txt
 +++ b/Documentation/devicetree/bindings/media/sunxi-ir.txt
 @@ -10,6 +10,7 @@ Required properties:

Optional properties:
- linux,rc-map-name : Remote control map name.
 +- resets : phandle + reset specifier pair



 Should it be optional? Or should we use a sun6i compatible with
 a mandatory reset phandle? I mean, the driver/hardware is not
 going to work with the reset missing on sun6i.

 Seems we are doing it one way for some of our drivers, and
 the other (optional) way for more generic ones, like USB.



 I do not believe that we should add a new compatible just because
 the reset line of a block is hooked up differently. It is the
 exact same ip-block. Only now the reset is not controlled
 through the apb-gate, but controlled separately.



 He has a point though. Your driver might very well probe nicely and
 everything, but still wouldn't be functional at all because the reset
 line wouldn't have been specified in the DT.



 Right, just like other drivers we've, see e.g.:

 Documentation/devicetree/bindings/mmc/sunxi-mmc.txt

 Which is dealing with this in the same way.

 The easiest way to deal with that would be in the bindings doc to
 update it with a compatible for the A31, and mentionning that the
 reset property is mandatory there.



 No the easiest way to deal with this is to expect people writing
 the dts to know what they are doing, just like we do for a lot
 of the other blocks in sun6i.

 Maybe put a generic note somewhere that sun6i has a reset controller,
 and that for all the blocks with optional resets property it should
 be considered mandatory on sun6i ?

 I'm sorry but I'm not going to make this change for the ir bindings
 given that we've the same situation in a lot of other places.

 Consistency is important. Moreover I believe that having a sun6i
 specific compatible string is just wrong, since it is the exact
 same hardware block as on sun5i, just with its reset line routed
 differently, just like e.g. the mmc controller, the uarts or the gmac
 all of which also do not have a sun6i specific compatible to enforce
 reset controller usage.

 Regards,

 Hans




 Note that the code itself might not change at all though. I'd just
 like to avoid any potential breaking of the DT bindings themselves. If
 we further want to refine the code, we can do that however we want.

 I have a slight preference for a clean error if reset is missing, but
 I won't get in the way just for that.


 Seems this patch and the following patch were overlooked after the
 discussion. Any chance we could get this in?


 I'm a linux/media sub-maintainer, so I've already send a pull-req for
 these 2 to the linux/media maintainer, iow this is taken care of :)

That's good to hear. I was going through the mainlining effort page,
and couldn't find these 2 in linux-next. I'll mark them as planned
for 3.20 then.


Chen-Yu
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [linux-sunxi] [PATCH v2 04/13] rc: sunxi-cir: Add support for an optional reset controller

2014-12-17 Thread Chen-Yu Tsai 
Hi,

On Thu, Dec 18, 2014 at 1:18 AM, Hans de Goede hdego...@redhat.com wrote:
 On sun6i the cir block is attached to the reset controller, add support
 for de-asserting the reset if a reset controller is specified in dt.

 Signed-off-by: Hans de Goede hdego...@redhat.com
 Acked-by: Mauro Carvalho Chehab mche...@osg.samsung.com
 Acked-by: Maxime Ripard maxime.rip...@free-electrons.com
 ---
  .../devicetree/bindings/media/sunxi-ir.txt |  2 ++
  drivers/media/rc/sunxi-cir.c   | 25 
 --
  2 files changed, 25 insertions(+), 2 deletions(-)

 diff --git a/Documentation/devicetree/bindings/media/sunxi-ir.txt 
 b/Documentation/devicetree/bindings/media/sunxi-ir.txt
 index 23dd5ad..6b70b9b 100644
 --- a/Documentation/devicetree/bindings/media/sunxi-ir.txt
 +++ b/Documentation/devicetree/bindings/media/sunxi-ir.txt
 @@ -10,6 +10,7 @@ Required properties:

  Optional properties:
  - linux,rc-map-name : Remote control map name.
 +- resets : phandle + reset specifier pair

Should it be optional? Or should we use a sun6i compatible with
a mandatory reset phandle? I mean, the driver/hardware is not
going to work with the reset missing on sun6i.

Seems we are doing it one way for some of our drivers, and
the other (optional) way for more generic ones, like USB.

ChenYu

  Example:

 @@ -17,6 +18,7 @@ ir0: ir@01c21800 {
 compatible = allwinner,sun4i-a10-ir;
 clocks = apb0_gates 6, ir0_clk;
 clock-names = apb, ir;
 +   resets = apb0_rst 1;
 interrupts = 0 5 1;
 reg = 0x01C21800 0x40;
 linux,rc-map-name = rc-rc6-mce;
 diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
 index 340f7f5..06170e0 100644
 --- a/drivers/media/rc/sunxi-cir.c
 +++ b/drivers/media/rc/sunxi-cir.c
 @@ -23,6 +23,7 @@
  #include linux/interrupt.h
  #include linux/module.h
  #include linux/of_platform.h
 +#include linux/reset.h
  #include media/rc-core.h

  #define SUNXI_IR_DEV sunxi-ir
 @@ -95,6 +96,7 @@ struct sunxi_ir {
 int irq;
 struct clk  *clk;
 struct clk  *apb_clk;
 +   struct reset_control *rst;
 const char  *map_name;
  };

 @@ -166,15 +168,29 @@ static int sunxi_ir_probe(struct platform_device *pdev)
 return PTR_ERR(ir-clk);
 }

 +   /* Reset (optional) */
 +   ir-rst = devm_reset_control_get_optional(dev, NULL);
 +   if (IS_ERR(ir-rst)) {
 +   ret = PTR_ERR(ir-rst);
 +   if (ret == -EPROBE_DEFER)
 +   return ret;
 +   ir-rst = NULL;
 +   } else {
 +   ret = reset_control_deassert(ir-rst);
 +   if (ret)
 +   return ret;
 +   }
 +
 ret = clk_set_rate(ir-clk, SUNXI_IR_BASE_CLK);
 if (ret) {
 dev_err(dev, set ir base clock failed!\n);
 -   return ret;
 +   goto exit_reset_assert;
 }

 if (clk_prepare_enable(ir-apb_clk)) {
 dev_err(dev, try to enable apb_ir_clk failed\n);
 -   return -EINVAL;
 +   ret = -EINVAL;
 +   goto exit_reset_assert;
 }

 if (clk_prepare_enable(ir-clk)) {
 @@ -271,6 +287,9 @@ exit_clkdisable_clk:
 clk_disable_unprepare(ir-clk);
  exit_clkdisable_apb_clk:
 clk_disable_unprepare(ir-apb_clk);
 +exit_reset_assert:
 +   if (ir-rst)
 +   reset_control_assert(ir-rst);

 return ret;
  }
 @@ -282,6 +301,8 @@ static int sunxi_ir_remove(struct platform_device *pdev)

 clk_disable_unprepare(ir-clk);
 clk_disable_unprepare(ir-apb_clk);
 +   if (ir-rst)
 +   reset_control_assert(ir-rst);

 spin_lock_irqsave(ir-ir_lock, flags);
 /* disable IR IRQ */
 --
 2.1.0

 --
 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.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/9] clk: sunxi: Add prcm mod0 clock driver

2014-11-27 Thread Chen-Yu Tsai 
Hi,

On Thu, Nov 27, 2014 at 4:41 PM, Hans de Goede hdego...@redhat.com wrote:
 Hi,


 On 11/26/2014 10:13 PM, Maxime Ripard wrote:

 Hi,

 On Tue, Nov 25, 2014 at 09:29:21AM +0100, Hans de Goede wrote:

 Hi,

 On 11/24/2014 11:03 PM, Maxime Ripard wrote:

 On Fri, Nov 21, 2014 at 10:13:10AM +0100, Hans de Goede wrote:

 Hi,

 On 11/21/2014 09:49 AM, Maxime Ripard wrote:

 Hi,

 On Thu, Nov 20, 2014 at 04:55:22PM +0100, Hans de Goede wrote:

 Add a driver for mod0 clocks found in the prcm. Currently there is
 only
 one mod0 clocks in the prcm, the ir clock.

 Signed-off-by: Hans de Goede hdego...@redhat.com
 ---
   Documentation/devicetree/bindings/clock/sunxi.txt |  1 +
   drivers/clk/sunxi/Makefile|  2 +-
   drivers/clk/sunxi/clk-sun6i-prcm-mod0.c   | 63
 +++
   drivers/mfd/sun6i-prcm.c  | 14 +
   4 files changed, 79 insertions(+), 1 deletion(-)
   create mode 100644 drivers/clk/sunxi/clk-sun6i-prcm-mod0.c

 diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt
 b/Documentation/devicetree/bindings/clock/sunxi.txt
 index ed116df..342c75a 100644
 --- a/Documentation/devicetree/bindings/clock/sunxi.txt
 +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
 @@ -56,6 +56,7 @@ Required properties:
 allwinner,sun4i-a10-usb-clk - for usb gates + resets on A10
 / A20
 allwinner,sun5i-a13-usb-clk - for usb gates + resets on A13
 allwinner,sun6i-a31-usb-clk - for usb gates + resets on A31
 +   allwinner,sun6i-a31-ir-clk - for the ir clock on A31

   Required properties for all clocks:
   - reg : shall be the control register address for the clock.
 diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
 index 7ddc2b5..daf8b1c 100644
 --- a/drivers/clk/sunxi/Makefile
 +++ b/drivers/clk/sunxi/Makefile
 @@ -10,4 +10,4 @@ obj-y += clk-sun8i-mbus.o

   obj-$(CONFIG_MFD_SUN6I_PRCM) += \
 clk-sun6i-ar100.o clk-sun6i-apb0.o clk-sun6i-apb0-gates.o \
 -   clk-sun8i-apb0.o
 +   clk-sun8i-apb0.o clk-sun6i-prcm-mod0.o
 diff --git a/drivers/clk/sunxi/clk-sun6i-prcm-mod0.c
 b/drivers/clk/sunxi/clk-sun6i-prcm-mod0.c
 new file mode 100644
 index 000..e80f18e
 --- /dev/null
 +++ b/drivers/clk/sunxi/clk-sun6i-prcm-mod0.c
 @@ -0,0 +1,63 @@
 +/*
 + * Allwinner A31 PRCM mod0 clock driver
 + *
 + * Copyright (C) 2014 Hans de Goede hdego...@redhat.com
 + *
 + * This program is free software; you can redistribute it and/or
 modify
 + * it under the terms of the GNU General Public License as published
 by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + */
 +
 +#include linux/clk-provider.h
 +#include linux/clkdev.h
 +#include linux/module.h
 +#include linux/of_address.h
 +#include linux/platform_device.h
 +
 +#include clk-factors.h
 +#include clk-mod0.h
 +
 +static const struct of_device_id sun6i_a31_prcm_mod0_clk_dt_ids[] =
 {
 +   { .compatible = allwinner,sun6i-a31-ir-clk },
 +   { /* sentinel */ }
 +};
 +
 +static DEFINE_SPINLOCK(sun6i_prcm_mod0_lock);
 +
 +static int sun6i_a31_prcm_mod0_clk_probe(struct platform_device
 *pdev)
 +{
 +   struct device_node *np = pdev-dev.of_node;
 +   struct resource *r;
 +   void __iomem *reg;
 +
 +   if (!np)
 +   return -ENODEV;
 +
 +   r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 +   reg = devm_ioremap_resource(pdev-dev, r);
 +   if (IS_ERR(reg))
 +   return PTR_ERR(reg);
 +
 +   sunxi_factors_register(np, sun4i_a10_mod0_data,
 +  sun6i_prcm_mod0_lock, reg);
 +   return 0;
 +}
 +
 +static struct platform_driver sun6i_a31_prcm_mod0_clk_driver = {
 +   .driver = {
 +   .name = sun6i-a31-prcm-mod0-clk,
 +   .of_match_table = sun6i_a31_prcm_mod0_clk_dt_ids,
 +   },
 +   .probe = sun6i_a31_prcm_mod0_clk_probe,
 +};
 +module_platform_driver(sun6i_a31_prcm_mod0_clk_driver);
 +
 +MODULE_DESCRIPTION(Allwinner A31 PRCM mod0 clock driver);
 +MODULE_AUTHOR(Hans de Goede hdego...@redhat.com);
 +MODULE_LICENSE(GPL);


 I don't think this is the right approach, mainly for two reasons: the
 compatible shouldn't change, and you're basically duplicating code
 there.

 I understand that you need the new compatible in order to avoid a
 double probing: one by CLK_OF_DECLARE, and one by the usual mechanism,
 and that also implies the second reason.


 Not only for that, we need a new compatible also because the mfd
 framework
 needs a separate compatible per sub-node as that is how it finds nodes
 to attach to the platform devices, so we need a new compatible anyways,
 with your make the mod0 clock driver a

Re: [linux-sunxi] [PATCH 3/9] clk: sunxi: Add prcm mod0 clock driver

2014-11-20 Thread Chen-Yu Tsai 
Hi,

On Thu, Nov 20, 2014 at 7:55 AM, Hans de Goede hdego...@redhat.com wrote:
 Add a driver for mod0 clocks found in the prcm. Currently there is only
 one mod0 clocks in the prcm, the ir clock.

 Signed-off-by: Hans de Goede hdego...@redhat.com
 ---
  Documentation/devicetree/bindings/clock/sunxi.txt |  1 +
  drivers/clk/sunxi/Makefile|  2 +-
  drivers/clk/sunxi/clk-sun6i-prcm-mod0.c   | 63 
 +++
  drivers/mfd/sun6i-prcm.c  | 14 +
  4 files changed, 79 insertions(+), 1 deletion(-)
  create mode 100644 drivers/clk/sunxi/clk-sun6i-prcm-mod0.c

 diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt 
 b/Documentation/devicetree/bindings/clock/sunxi.txt
 index ed116df..342c75a 100644
 --- a/Documentation/devicetree/bindings/clock/sunxi.txt
 +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
 @@ -56,6 +56,7 @@ Required properties:
 allwinner,sun4i-a10-usb-clk - for usb gates + resets on A10 / A20
 allwinner,sun5i-a13-usb-clk - for usb gates + resets on A13
 allwinner,sun6i-a31-usb-clk - for usb gates + resets on A31
 +   allwinner,sun6i-a31-ir-clk - for the ir clock on A31

  Required properties for all clocks:
  - reg : shall be the control register address for the clock.
 diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
 index 7ddc2b5..daf8b1c 100644
 --- a/drivers/clk/sunxi/Makefile
 +++ b/drivers/clk/sunxi/Makefile
 @@ -10,4 +10,4 @@ obj-y += clk-sun8i-mbus.o

  obj-$(CONFIG_MFD_SUN6I_PRCM) += \
 clk-sun6i-ar100.o clk-sun6i-apb0.o clk-sun6i-apb0-gates.o \
 -   clk-sun8i-apb0.o
 +   clk-sun8i-apb0.o clk-sun6i-prcm-mod0.o
 diff --git a/drivers/clk/sunxi/clk-sun6i-prcm-mod0.c 
 b/drivers/clk/sunxi/clk-sun6i-prcm-mod0.c
 new file mode 100644
 index 000..e80f18e
 --- /dev/null
 +++ b/drivers/clk/sunxi/clk-sun6i-prcm-mod0.c
 @@ -0,0 +1,63 @@
 +/*
 + * Allwinner A31 PRCM mod0 clock driver
 + *
 + * Copyright (C) 2014 Hans de Goede hdego...@redhat.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + */
 +
 +#include linux/clk-provider.h
 +#include linux/clkdev.h
 +#include linux/module.h
 +#include linux/of_address.h
 +#include linux/platform_device.h
 +
 +#include clk-factors.h
 +#include clk-mod0.h
 +
 +static const struct of_device_id sun6i_a31_prcm_mod0_clk_dt_ids[] = {
 +   { .compatible = allwinner,sun6i-a31-ir-clk },

Could we use a generic name, like sun6i-a31-prcm-mod0-clk?
IIRC, there is another one, the module clock for the 1-wire interface.

Same for the DT patches.

ChenYu

 +   { /* sentinel */ }
 +};
 +
 +static DEFINE_SPINLOCK(sun6i_prcm_mod0_lock);
 +
 +static int sun6i_a31_prcm_mod0_clk_probe(struct platform_device *pdev)
 +{
 +   struct device_node *np = pdev-dev.of_node;
 +   struct resource *r;
 +   void __iomem *reg;
 +
 +   if (!np)
 +   return -ENODEV;
 +
 +   r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 +   reg = devm_ioremap_resource(pdev-dev, r);
 +   if (IS_ERR(reg))
 +   return PTR_ERR(reg);
 +
 +   sunxi_factors_register(np, sun4i_a10_mod0_data,
 +  sun6i_prcm_mod0_lock, reg);
 +   return 0;
 +}
 +
 +static struct platform_driver sun6i_a31_prcm_mod0_clk_driver = {
 +   .driver = {
 +   .name = sun6i-a31-prcm-mod0-clk,
 +   .of_match_table = sun6i_a31_prcm_mod0_clk_dt_ids,
 +   },
 +   .probe = sun6i_a31_prcm_mod0_clk_probe,
 +};
 +module_platform_driver(sun6i_a31_prcm_mod0_clk_driver);
 +
 +MODULE_DESCRIPTION(Allwinner A31 PRCM mod0 clock driver);
 +MODULE_AUTHOR(Hans de Goede hdego...@redhat.com);
 +MODULE_LICENSE(GPL);
 diff --git a/drivers/mfd/sun6i-prcm.c b/drivers/mfd/sun6i-prcm.c
 index 283ab8d..ff1254f 100644
 --- a/drivers/mfd/sun6i-prcm.c
 +++ b/drivers/mfd/sun6i-prcm.c
 @@ -41,6 +41,14 @@ static const struct resource 
 sun6i_a31_apb0_gates_clk_res[] = {
 },
  };

 +static const struct resource sun6i_a31_ir_clk_res[] = {
 +   {
 +   .start = 0x54,
 +   .end = 0x57,
 +   .flags = IORESOURCE_MEM,
 +   },
 +};
 +
  static const struct resource sun6i_a31_apb0_rstc_res[] = {
 {
 .start = 0xb0,
 @@ -69,6 +77,12 @@ static const struct mfd_cell sun6i_a31_prcm_subdevs[] = {
 .resources = sun6i_a31_apb0_gates_clk_res,
 },
 {
 +   .name = sun6i-a31-ir-clk,
 +   .of_compatible =