Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH V4 1/9] PM / OPP: Allow OPP table to be used for power-domains
From: Viresh Kumar @ 2017-04-13  5:50 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Rafael Wysocki, ulf.hansson, Kevin Hilman, Viresh Kumar,
	Nishanth Menon, Stephen Boyd, linaro-kernel, linux-pm,
	linux-kernel, Vincent Guittot, robh+dt, lina.iyer, rnayak,
	devicetree
In-Reply-To: <468e756b-7112-4006-b31d-9fcf1c32673d@arm.com>

On 12-04-17, 18:05, Sudeep Holla wrote:
> 
> 
> On 20/03/17 09:32, Viresh Kumar wrote:
> [...]
> 
> > +
> > +Example 7: domain-Performance-state:
> > +(example: For 1GHz require domain state 1 and for 1.1 & 1.2 GHz require state 2)
> > +
> > +/ {
> > +	domain_opp_table: opp_table0 {
> > +		compatible = "operating-points-v2";
> > +
> > +		opp@1 {
> > +			domain-performance-state = <1>;
> > +			opp-microvolt = <975000 970000 985000>;
> > +		};
> > +		opp@2 {
> > +			domain-performance-state = <2>;
> > +			opp-microvolt = <1075000 1000000 1085000>;
> > +		};
> > +	};
> > +
> > +	foo_domain: power-controller@12340000 {
> > +		compatible = "foo,power-controller";
> > +		reg = <0x12340000 0x1000>;
> > +		#power-domain-cells = <0>;
> > +		operating-points-v2 = <&domain_opp_table>;
> > +	}
> > +
> > +	cpu0_opp_table: opp_table1 {
> > +		compatible = "operating-points-v2";
> > +		opp-shared;
> > +
> > +		opp@1000000000 {
> > +			opp-hz = /bits/ 64 <1000000000>;
> > +			domain-performance-state = <1>;
> > +		};
> > +		opp@1100000000 {
> > +			opp-hz = /bits/ 64 <1100000000>;
> > +			domain-performance-state = <2>;
> > +		};
> > +		opp@1200000000 {
> > +			opp-hz = /bits/ 64 <1200000000>;
> > +			domain-performance-state = <2>;
> > +		};
> > +	};
> > +
> > +	cpus {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		cpu@0 {
> > +			compatible = "arm,cortex-a9";
> > +			reg = <0>;
> > +			clocks = <&clk_controller 0>;
> > +			clock-names = "cpu";
> > +			operating-points-v2 = <&cpu0_opp_table>;
> > +			power-domains = <&foo_domain>;
> > +		};
> > +	};
> > +};
> 
> 
> Thinking more about this above example, I think you need more
> explanation. So in the above case you have cpu with clock controller,
> power-domain and the OPP table info, I can think of few things that need
> to be explicit:
> 
> 1. How does the precedence look like ?

Just think of the power-domain as a regulator here. If we are
increasing frequency of the device, power-domain needs to be
programmed first followed by the clock.

> 2. Since power-domains with OPP table control the performance state, do

They control performance state of the domains, not the devices.

>    we ignore clock and operating-points-v2 in the above case completely?

No. They are separate.

> 
> 3. Will the power-domain drive the OPP ?

power-domain will driver its own state using its own OPP table.
Devices may fine tune within those states.

-- 
viresh

^ permalink raw reply

* Re: [PATCH V4 1/9] PM / OPP: Allow OPP table to be used for power-domains
From: Viresh Kumar @ 2017-04-13  5:37 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Rafael Wysocki, ulf.hansson-QSEj5FYQhm4dnm+yROfE0A, Kevin Hilman,
	Viresh Kumar, Nishanth Menon, Stephen Boyd,
	linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, lina.iyer-QSEj5FYQhm4dnm+yROfE0A,
	rnayak-sgV2jX0FEOL9JmXXK+q4OQ, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <0a7146f9-72f1-317c-3aab-770a72462968-5wv7dgnIgG8@public.gmane.org>

On 12-04-17, 17:49, Sudeep Holla wrote:
> On 20/03/17 09:32, Viresh Kumar wrote:
> > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> > index 63725498bd20..d0b95c9e1011 100644
> > --- a/Documentation/devicetree/bindings/opp/opp.txt
> > +++ b/Documentation/devicetree/bindings/opp/opp.txt
> > @@ -76,10 +76,9 @@ This describes the OPPs belonging to a device. This node can have following
> >  This defines voltage-current-frequency combinations along with other related
> >  properties.
> >  
> > -Required properties:
> > +Optional properties:
> >  - opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer.
> >  
> > -Optional properties:
> >  - opp-microvolt: voltage in micro Volts.
> >  
> >    A single regulator's voltage is specified with an array of size one or three.
> > @@ -154,6 +153,19 @@ properties.
> >  
> >  - status: Marks the node enabled/disabled.
> >  
> > +- domain-performance-state: A positive integer value representing the minimum
> > +  power-domain performance level required by the device for the OPP node. The
> 
> So the above definition is when this field in in the device node rather
> than the OPP table entry, right ?

No. We are updating the opp.txt file here and so it is not about the
device node. The OPP node entries will contain this field for two
cases:
- The OPP table belongs to a power domain
- The OPP table belongs to a device whose power domain supports
  performance-states.

> For simplicity why not have the
> properties named slightly different or just use phandle to an entry in
> the device node for this purpose.

We really need a value here. For example, in case where the OPP table
defines the states of the power-domain itself, we don't have any
phandles to point to.

> > +  The integer value '0' represents the lowest performance level and the higher
> > +  values represent higher performance levels. 
> 
> needs to be changed as OPP table entry.

Not sure I understood what change you are looking for :(

> >  When present in the OPP table of a
> > + power-domain, it represents the performance level of the domain. When present
> 
> again "performance level of the domain corresponding to that OPP entry"
> on something similar

Ok.

> > +  in the OPP table of a normal device, it represents the performance level of
> 
> what do you mean by normal device ? needs description as that's
> something new introduced here.

It should be non-power-domain node.

> > +  the parent power-domain. The OPP table can contain the
> > +  "domain-performance-state" property, only if the device node contains the
> > +  "power-domains" or "#power-domain-cells" property. 
> 
> Why such a restriction ?

Why would we use it for non-power-domain cases? That's not what we
are looking for..

> > The OPP nodes aren't
> > +  allowed to contain the "domain-performance-state" property partially, i.e.
> > +  Either all OPP nodes in the OPP table have the "domain-performance-state"
> > +  property or none of them have it.
> > +
> >  Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together.
> >  
> >  / {
> > @@ -528,3 +540,60 @@ Example 5: opp-supported-hw
> >  		};
> >  	};
> >  };
> > +
> > +Example 7: domain-Performance-state:
> > +(example: For 1GHz require domain state 1 and for 1.1 & 1.2 GHz require state 2)
> > +
> > +/ {
> > +	domain_opp_table: opp_table0 {
> > +		compatible = "operating-points-v2";
> > +
> > +		opp@1 {
> > +			domain-performance-state = <1>;
> > +			opp-microvolt = <975000 970000 985000>;
> > +		};
> > +		opp@2 {
> > +			domain-performance-state = <2>;
> > +			opp-microvolt = <1075000 1000000 1085000>;
> > +		};
> > +	};
> > +
> > +	foo_domain: power-controller@12340000 {
> > +		compatible = "foo,power-controller";
> > +		reg = <0x12340000 0x1000>;
> > +		#power-domain-cells = <0>;
> > +		operating-points-v2 = <&domain_opp_table>;
> 
> How does it scale with power domain providers with multiple power domain ?

Devices can't have multiple power domains today. Will see this when
that support is added.

Note that only the power domains can have multiple parent power
domains today.

> > +	}
> > +
> > +	cpu0_opp_table: opp_table1 {
> > +		compatible = "operating-points-v2";
> > +		opp-shared;
> > +
> > +		opp@1000000000 {
> > +			opp-hz = /bits/ 64 <1000000000>;
> > +			domain-performance-state = <1>;
> > +		};
> > +		opp@1100000000 {
> > +			opp-hz = /bits/ 64 <1100000000>;
> > +			domain-performance-state = <2>;
> > +		};
> > +		opp@1200000000 {
> > +			opp-hz = /bits/ 64 <1200000000>;
> > +			domain-performance-state = <2>;
> > +		};
> > +	};
> > +
> > +	cpus {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		cpu@0 {
> > +			compatible = "arm,cortex-a9";
> > +			reg = <0>;
> > +			clocks = <&clk_controller 0>;
> > +			clock-names = "cpu";
> > +			operating-points-v2 = <&cpu0_opp_table>;
> 
> Do we ignore operating-points-v2 above as this device/cpu node contains
> power domain which has operating-points-v2 property ? In other words
> how do they correlate ?

Devices and their power domains can both have their performance
states. Just that to get the device in a particular state, we may need
to get its power domain to a particular state first.

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] ARM: dts: dra7: Add power hold and power controller properties to palmas
From: Keerthy @ 2017-04-13  4:51 UTC (permalink / raw)
  To: tony-4v6yS6AI5VpBDgjK7y7TUQ
  Cc: j-keerthy-l0cyMroinI0, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA, t-kristo-l0cyMroinI0

Add power hold and power controller properties to palmas node.
This is needed to shutdown pmic correctly on boards with
powerhold set.

Signed-off-by: Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org>
---
 arch/arm/boot/dts/dra7-evm.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
index 4bc4b57..31a9e06 100644
--- a/arch/arm/boot/dts/dra7-evm.dts
+++ b/arch/arm/boot/dts/dra7-evm.dts
@@ -204,6 +204,8 @@
 	tps659038: tps659038@58 {
 		compatible = "ti,tps659038";
 		reg = <0x58>;
+		ti,palmas-override-powerhold;
+		ti,system-power-controller;
 
 		tps659038_pmic {
 			compatible = "ti,tps659038-pmic";
-- 
1.9.1

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

^ permalink raw reply related

* [PATCH] [media] mtk-mdp: Fix g_/s_selection capture/compose logic
From: Minghsiu Tsai @ 2017-04-13  4:18 UTC (permalink / raw)
  To: Hans Verkuil, daniel.thompson-QSEj5FYQhm4dnm+yROfE0A, Rob Herring,
	Mauro Carvalho Chehab, Matthias Brugger, Daniel Kurtz,
	Pawel Osciak, Houlong Wei
  Cc: srv_heupstream-NuS5LvNUpcJWk0Htik3J/w, Eddie Huang, Yingjoe Chen,
	Wu-Cheng Li, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Minghsiu Tsai

From: Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Experiments show that the:
 (1) mtk-mdp uses the _MPLANE form of CAPTURE/OUTPUT
 (2) CAPTURE types use CROP targets, and OUTPUT types use COMPOSE targets

Signed-off-by: Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Minghsiu Tsai <minghsiu.tsai-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

---
 drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
index 13afe48..8ab7ca0 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
@@ -837,12 +837,12 @@ static int mtk_mdp_m2m_g_selection(struct file *file, void *fh,
 	struct mtk_mdp_ctx *ctx = fh_to_ctx(fh);
 	bool valid = false;
 
-	if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
-		if (mtk_mdp_is_target_compose(s->target))
-			valid = true;
-	} else if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
+	if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
 		if (mtk_mdp_is_target_crop(s->target))
 			valid = true;
+	} else if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
+		if (mtk_mdp_is_target_compose(s->target))
+			valid = true;
 	}
 	if (!valid) {
 		mtk_mdp_dbg(1, "[%d] invalid type:%d,%u", ctx->id, s->type,
@@ -907,12 +907,12 @@ static int mtk_mdp_m2m_s_selection(struct file *file, void *fh,
 	int ret;
 	bool valid = false;
 
-	if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
-		if (s->target == V4L2_SEL_TGT_COMPOSE)
-			valid = true;
-	} else if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
+	if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
 		if (s->target == V4L2_SEL_TGT_CROP)
 			valid = true;
+	} else if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
+		if (s->target == V4L2_SEL_TGT_COMPOSE)
+			valid = true;
 	}
 	if (!valid) {
 		mtk_mdp_dbg(1, "[%d] invalid type:%d,%u", ctx->id, s->type,
@@ -925,7 +925,7 @@ static int mtk_mdp_m2m_s_selection(struct file *file, void *fh,
 	if (ret)
 		return ret;
 
-	if (mtk_mdp_is_target_crop(s->target))
+	if (mtk_mdp_is_target_compose(s->target))
 		frame = &ctx->s_frame;
 	else
 		frame = &ctx->d_frame;
-- 
1.9.1

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

^ permalink raw reply related

* Re: [PATCH v3] arm64: dts: rk3399: add support for firefly-rk3399 board
From: Kever Yang @ 2017-04-13  4:00 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Jianqun Xu, Liang Chen,
	Brian Norris, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andy Yan,
	Rob Herring, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Will Deacon, Mark Rutland, Catalin Marinas, Matthias Brugger
In-Reply-To: <1642127.PiTFsmK4zh@phil>

Hi Heiko,


On 04/12/2017 09:29 PM, Heiko Stuebner wrote:
> Hi Kever,
>
> Am Montag, 10. April 2017, 11:50:13 CEST schrieb Kever Yang:
>> Firefly-rk3399 is a bord from T-Firefly, you can find detail about
>> it here:
>> http://en.t-firefly.com/en/firenow/Firefly_RK3399/
>>
>> This patch add basic node for the board and make it able to bring
>> up.
>>
>> Peripheral works:
>> - usb hub which connect to ehci controller;
>> - UART2 debug
>> - eMMC
>> - PCIe
>>
>> Not work:
>> - USB 3.0 HOST, type-C port
>> - sdio, sd-card
>>
>> Not test for other peripheral:
>> - HDMI
>> - Ethernet
>> - OPTICAL
>> - WiFi/BT
>> - MIPI CSI/DSI
>> - IR
>> - EDP/DP
>>
>> Signed-off-by: Kever Yang <kever.yang-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> applied for 4.13, as we're a bit late for 4.12, with the following changes:

Thanks for your help, I'm not familiar with the devices status on upstream
because not working on kernel upstream for a long time.
> - commit subject
> - dropped status from backlight (as there is no disabled common node
>    and it's specific to the firefly itself)
> - quite some reordering of properties
> - reordered regulator nodes per their addresses: 0x1b < 0x40
> - dropped obsolete regulator-compatible properties
> - fixed gpio-irq on the mpu6500
> - dropped out-of-tree orientation properties of mpu6500
>    --> please provide the optional "mount-matrix" in a follow-up patch
>        see bindings/iio/imu/inv_mpu6050.txt

Maybe we can drop this mpu6050 node first? I can't find binding file for it.
> - dropped rockchip,i2s-broken-burst-len;
>    That change never made it into the mainline kernel
> - fixed pcie pinctrl indentation
> - dropped wireless-bluetooth uart-gpios pinctrl
> - dropped supports-emmc property
>
> Please try to be a bit more careful when porting stuff from device kernels
> with respect to properties not found in the mainline kernel and please
> also double-check in [0] that I didn't break anything.

I have test this patch on my firefly-rk3399, it works fine with pwm2 
regulator
init in U-Boot.

Thanks,
- Kever
>
>
> Thanks
> Heiko
>
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git/commit/?id=495a3c891a696b62465d71b1a125e3424352028b
>
>


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

^ permalink raw reply

* Re: [PATCH v2 0/3] of: Make of_match_node() an inline stub for CONFIG_OF=n
From: Frank Rowand @ 2017-04-13  3:46 UTC (permalink / raw)
  To: Florian Fainelli, linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: andrew-g2DYL2Zd6BY,
	vivien.didelot-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/, Lee Jones,
	Nicolas Ferre, Rob Herring, open list:NETWORKING DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE
In-Reply-To: <20170412044156.17351-1-f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On 04/11/17 21:41, Florian Fainelli wrote:
> Hi all,
> 
> This patch series makes of_match_node() an inline stub for CONFIG_OF=n. kbuild
> reported two build errors which are fixed as preriquisite patches.
> 
> This is based on Linus' master, not sure which tree would merge this, Frank's?

It would come in via Rob.

I am not comfortable with patch 3/3 at this moment.

Version 1 of the patch resulted in two errors from the kbuild test robot.  This
version results in another error from the kbuild test robot.  I know it is a
lot of work, but please look at all of the callers of of_match_node() and
check whether any of the other callers will have the same type of error that
the kbuild test robot is catching.

-Frank

> 
> Thanks!
> 
> Florian Fainelli (3):
>   mfd: max8998: Remove CONFIG_OF around max8998_dt_match
>   net: macb: Remove CONFIG_OF around DT match table
>   of: Make of_match_node() an inline stub for CONFIG_OF=n
> 
>  drivers/mfd/max8998.c               | 2 --
>  drivers/net/ethernet/cadence/macb.c | 2 --
>  include/linux/of.h                  | 6 +++++-
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 

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

^ permalink raw reply

* Re: [PATCH v1 1/1] mtd: mtk-nor: set controller's address width according to nor flash
From: Guochun Mao @ 2017-04-13  2:40 UTC (permalink / raw)
  To: Cyrille Pitchen
  Cc: Cyrille Pitchen, Mark Rutland, devicetree, Richard Weinberger,
	Russell King, linux-kernel, Rob Herring, linux-mtd,
	Matthias Brugger, linux-mediatek, David Woodhouse,
	linux-arm-kernel
In-Reply-To: <e9eafca3-41a0-7e88-996b-9d1c51f0c132@wedev4u.fr>

Hi Cyrille,

On Wed, 2017-04-12 at 22:57 +0200, Cyrille Pitchen wrote:
> Hi Guochun,
> 
> Le 05/04/2017 à 10:37, Guochun Mao a écrit :
> > When nor's size larger than 16MByte, nor's address width maybe
> > set to 3 or 4, and controller should change address width according
> > to nor's setting.
> > 
> > Signed-off-by: Guochun Mao <guochun.mao@mediatek.com>st
> > ---
> >  drivers/mtd/spi-nor/mtk-quadspi.c |   27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> > 
> > diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c
> > index e661877..b637770 100644
> > --- a/drivers/mtd/spi-nor/mtk-quadspi.c
> > +++ b/drivers/mtd/spi-nor/mtk-quadspi.c
> > @@ -104,6 +104,8 @@
> >  #define MTK_NOR_MAX_RX_TX_SHIFT		6
> >  /* can shift up to 56 bits (7 bytes) transfer by MTK_NOR_PRG_CMD */
> >  #define MTK_NOR_MAX_SHIFT		7
> > +/* nor controller 4-byte address mode enable bit */
> > +#define MTK_NOR_4B_ADDR_EN		BIT(4)
> >  
> >  /* Helpers for accessing the program data / shift data registers */
> >  #define MTK_NOR_PRG_REG(n)		(MTK_NOR_PRGDATA0_REG + 4 * (n))
> > @@ -230,10 +232,35 @@ static int mt8173_nor_write_buffer_disable(struct mt8173_nor *mt8173_nor)
> >  				  10000);
> >  }
> >  
> > +static void mt8173_nor_set_addr_width(struct mt8173_nor *mt8173_nor)
> > +{
> > +	u8 val;
> > +	struct spi_nor *nor = &mt8173_nor->nor;
> > +
> > +	val = readb(mt8173_nor->base + MTK_NOR_DUAL_REG);
> > +
> > +	switch (nor->addr_width) {
> > +	case 3:
> > +		val &= ~MTK_NOR_4B_ADDR_EN;
> > +		break;
> > +	case 4:
> > +		val |= MTK_NOR_4B_ADDR_EN;
> > +		break;
> > +	default:
> > +		dev_warn(mt8173_nor->dev, "Unexpected address width %u.\n",
> > +			 nor->addr_width);
> > +		break;
> > +	}
> > +
> > +	writeb(val, mt8173_nor->base + MTK_NOR_DUAL_REG);
> > +}
> > +
> >  static void mt8173_nor_set_addr(struct mt8173_nor *mt8173_nor, u32 addr)
> >  {
> >  	int i;
> >  
> > +	mt8173_nor_set_addr_width(mt8173_nor);
> > +
> >  	for (i = 0; i < 3; i++) {
> 
> Should it be 'i < nor->addr_width' instead of 'i < 3' ?
> Does it work when accessing data after 128Mbit ?

Yes, it can work.

Let's see the whole function,

static void mt8173_nor_set_addr(struct mt8173_nor *mt8173_nor, u32 addr)
{
        int i;

        mt8173_nor_set_addr_width(mt8173_nor);

        for (i = 0; i < 3; i++) {
                writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR0_REG
+ i * 4);
                addr >>= 8;
        }
        /* Last register is non-contiguous */
        writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR3_REG);
}

The nor controller has 4 registers for address.
This '3' indicates the number of contiguous address' registers
base + MTK_NOR_RADR0_REG(0x10)
base + MTK_NOR_RADR1_REG(0x14)
base + MTK_NOR_RADR2_REG(0x18),
but the last address register is non-contiguous,
it's base + MTK_NOR_RADR3_REG(0xc8)

mt8173_nor_set_addr will set addr into these 4 registers by Byte.
The bit MTK_NOR_4B_ADDR_EN will decide whether 3-byte(0x10,0x14,0x18)
or 4-byte(0x10,0x14,x018,0xc8) been sent to nor device.
and, it can access data after 128Mbit when sent 4-byte address.

Best regards,

Guochun

> 
> Best regards,
> 
> Cyrille
> 
> >  		writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR0_REG + i * 4);
> >  		addr >>= 8;
> > 
> 

^ permalink raw reply

* Re: [PATCH] [media] imx: csi: retain current field order and colorimetry setting as default
From: Steve Longerbeam @ 2017-04-13  0:33 UTC (permalink / raw)
  To: Philipp Zabel, Russell King - ARM Linux
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	fabio.estevam-3arQi8VN3Tc, mchehab-DgEjT+Ai2ygdnm+yROfE0A,
	hverkuil-qWit8jRvyhVmR6Xm/wNWPw, nick-gcszYUEDH4VrovVCs/uTlw,
	markus.heiser-O6JHGLzbNUwb1SvskN2V4Q,
	laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw,
	bparrot-l0cyMroinI0, geert-Td1EMuHUCqxL1ZNQvxDV9g,
	arnd-r2nGTMty4D4, sudipm.mukherjee-Re5JQEeQqe8AvxtiuMwx3w,
	minghsiu.tsai-NuS5LvNUpcJWk0Htik3J/w,
	tiffany.lin-NuS5LvNUpcJWk0Htik3J/w,
	jean-christophe.trotin-qxv4g6HH51o,
	horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ,
	niklas.soderlund+renesas-1zkq55x86MTxsAP9Fp7wbw,
	robert.jarzmik-GANU6spQydw, songjun.wu-UWL1GkI3JZL3oGB3hsPCZA,
	andrew-ct.chen-NuS5LvNUpcJWk0Htik3J/w,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	shuah-DgEjT+Ai2ygdnm+yROfE0A, sakari.ailus-VuQAYsv1563Yd54FQh9/CA,
	pavel-+ZI9xUNit7I, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b
In-Reply-To: <1491492354.2392.87.camel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>



On 04/06/2017 08:25 AM, Philipp Zabel wrote:
> On Thu, 2017-04-06 at 16:10 +0100, Russell King - ARM Linux wrote:
>> On Thu, Apr 06, 2017 at 05:01:52PM +0200, Philipp Zabel wrote:
>>> On Thu, 2017-04-06 at 15:05 +0100, Russell King - ARM Linux wrote:
>>>> On Thu, Apr 06, 2017 at 03:55:29PM +0200, Philipp Zabel wrote:
>>>>> +
>>>>> +	/* Retain current field setting as default */
>>>>> +	if (sdformat->format.field == V4L2_FIELD_ANY)
>>>>> +		sdformat->format.field = fmt->field;
>>>>> +
>>>>> +	/* Retain current colorspace setting as default */
>>>>> +	if (sdformat->format.colorspace == V4L2_COLORSPACE_DEFAULT) {
>>>>> +		sdformat->format.colorspace = fmt->colorspace;
>>>>> +		if (sdformat->format.xfer_func == V4L2_XFER_FUNC_DEFAULT)
>>>>> +			sdformat->format.xfer_func = fmt->xfer_func;
>>>>> +		if (sdformat->format.ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT)
>>>>> +			sdformat->format.ycbcr_enc = fmt->ycbcr_enc;
>>>>> +		if (sdformat->format.quantization == V4L2_QUANTIZATION_DEFAULT)
>>>>> +			sdformat->format.quantization = fmt->quantization;
>>>>> +	} else {
>>>>> +		if (sdformat->format.xfer_func == V4L2_XFER_FUNC_DEFAULT) {
>>>>> +			sdformat->format.xfer_func =
>>>>> +				V4L2_MAP_XFER_FUNC_DEFAULT(
>>>>> +						sdformat->format.colorspace);
>>>>> +		}
>>>>> +		if (sdformat->format.ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT) {
>>>>> +			sdformat->format.ycbcr_enc =
>>>>> +				V4L2_MAP_YCBCR_ENC_DEFAULT(
>>>>> +						sdformat->format.colorspace);
>>>>> +		}
>>>>> +		if (sdformat->format.quantization == V4L2_QUANTIZATION_DEFAULT) {
>>>>> +			sdformat->format.quantization =
>>>>> +				V4L2_MAP_QUANTIZATION_DEFAULT(
>>>>> +						cc->cs != IPUV3_COLORSPACE_YUV,
>>>>> +						sdformat->format.colorspace,
>>>>> +						sdformat->format.ycbcr_enc);
>>>>> +		}
>>>>> +	}
>>>>
>>>> Would it make sense for this to be a helper function?
>>>
>>> Quite possible, the next subdev that has to set frame_interval on both
>>> pads manually because its upstream source pad doesn't suport
>>> frame_interval might want to do the same.
>>
>> Hmm.  I'm not sure I agree with this approach.  If a subdev hardware
>> does not support any modification of the colourspace or field, then
>> it should not be modifyable at the source pad - it should retain the
>> propagated settings from the sink pad.
>
> This new code is only relevant for the CSI_SINK_PAD.
>
>> I thought I had already sent a patch doing exactly that.
>
> Yes. Right above the modification there is a call to csi_try_fmt which
> will already fix up sdformat->format for the source pads. So for the
> CSI_SRC_PAD_DIRECT and CSI_SRC_PAD_IDMAC this should amount to a no-op.
>
> If might be better to move this into a separate function and only call
> it if sdformat->pad == CSI_SINK_PAD.

I've done this, I will follow with the new patch. Philipp, let me know
if this looks ok to you and I will add your sign-off.

Steve


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

^ permalink raw reply

* [PATCH v3] ARM: dts: stm32f7: add stm32f769I & stm32f746 discovery board support
From: Vikas Manocha @ 2017-04-13  0:27 UTC (permalink / raw)
  To: alexandre.torgue-qxv4g6HH51o, patrice.chotard-qxv4g6HH51o
  Cc: Vikas Manocha,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM PORT, open list, Mark Rutland, Maxime Coquelin,
	Rob Herring, Russell King

Stm32f769I & stm32f746 are MCUs of stm32f7 family. Here are the major
specs of the two boards:

stm32f769I discovery board:
	- Cortex-M7 core @216MHz
	- 2MB mcu internal flash
	- 512KB internal sram
	- 16MB sdram memory
	- 64MB qspi flash memory
	- 4 inch wvga LCD-TFT Display

stm32f746 discovery board:
	- Cortex-M7 core @216MHz
	- 1MB mcu internal flash
	- 320KB internal sram
	- 8MB sdram memory
	- 16MB qspi flash memory
	- 4.3 inch 480x272 LCD-TFT display

Signed-off-by: Vikas Manocha <vikas.manocha-qxv4g6HH51o@public.gmane.org>
---
Changed in v3:
- fixed usart pin muxing for stm32f746 board, correct one is usart1_pins_pa9_pb7.
- Added patch version in the patch title, was missed in v2.

Changed in v2:
- moved pin muxing from board dts file to soc dtsi

 arch/arm/boot/dts/Makefile            |  2 +
 arch/arm/boot/dts/stm32f746-disco.dts | 74 +++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/stm32f746.dtsi      | 12 ++++++
 arch/arm/boot/dts/stm32f769-disco.dts | 74 +++++++++++++++++++++++++++++++++++
 4 files changed, 162 insertions(+)
 create mode 100644 arch/arm/boot/dts/stm32f746-disco.dts
 create mode 100644 arch/arm/boot/dts/stm32f769-disco.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 0118084..a119f74 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -763,6 +763,8 @@ dtb-$(CONFIG_ARCH_STI) += \
 dtb-$(CONFIG_ARCH_STM32)+= \
 	stm32f429-disco.dtb \
 	stm32f469-disco.dtb \
+	stm32f746-disco.dtb \
+	stm32f769-disco.dtb \
 	stm32429i-eval.dtb \
 	stm32746g-eval.dtb
 dtb-$(CONFIG_MACH_SUN4I) += \
diff --git a/arch/arm/boot/dts/stm32f746-disco.dts b/arch/arm/boot/dts/stm32f746-disco.dts
new file mode 100644
index 0000000..52e5834
--- /dev/null
+++ b/arch/arm/boot/dts/stm32f746-disco.dts
@@ -0,0 +1,74 @@
+/*
+ * Copyright 2017 - Vikas MANOCHA <vikas.manocha-qxv4g6HH51o@public.gmane.org>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file 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 file 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.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "stm32f746.dtsi"
+#include <dt-bindings/input/input.h>
+
+/ {
+	model = "STMicroelectronics STM32F746-DISCO board";
+	compatible = "st,stm32f746-disco", "st,stm32f746";
+
+	chosen {
+		bootargs = "root=/dev/ram";
+		stdout-path = "serial0:115200n8";
+	};
+
+	memory {
+		reg = <0xC0000000 0x800000>;
+	};
+
+	aliases {
+		serial0 = &usart1;
+	};
+
+};
+
+&clk_hse {
+	clock-frequency = <25000000>;
+};
+
+&usart1 {
+	pinctrl-0 = <&usart1_pins_pa9_pb7>;
+	pinctrl-names = "default";
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/stm32f746.dtsi b/arch/arm/boot/dts/stm32f746.dtsi
index f321ffe..82becbb 100644
--- a/arch/arm/boot/dts/stm32f746.dtsi
+++ b/arch/arm/boot/dts/stm32f746.dtsi
@@ -287,6 +287,18 @@
 					bias-disable;
 				};
 			};
+			usart1_pins_pa9_pb7: usart1@1 {
+				pins1 {
+					pinmux = <STM32F746_PA9_FUNC_USART1_TX>;
+					bias-disable;
+					drive-push-pull;
+					slew-rate = <0>;
+				};
+				pins2 {
+					pinmux = <STM32F746_PB7_FUNC_USART1_RX>;
+					bias-disable;
+				};
+			};
 		};
 
 		rcc: rcc@40023800 {
diff --git a/arch/arm/boot/dts/stm32f769-disco.dts b/arch/arm/boot/dts/stm32f769-disco.dts
new file mode 100644
index 0000000..166728a
--- /dev/null
+++ b/arch/arm/boot/dts/stm32f769-disco.dts
@@ -0,0 +1,74 @@
+/*
+ * Copyright 2017 - Vikas MANOCHA <vikas.manocha-qxv4g6HH51o@public.gmane.org>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file 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 file 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.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "stm32f746.dtsi"
+#include <dt-bindings/input/input.h>
+
+/ {
+	model = "STMicroelectronics STM32F769-DISCO board";
+	compatible = "st,stm32f769-disco", "st,stm32f7";
+
+	chosen {
+		bootargs = "root=/dev/ram";
+		stdout-path = "serial0:115200n8";
+	};
+
+	memory {
+		reg = <0xC0000000 0x1000000>;
+	};
+
+	aliases {
+		serial0 = &usart1;
+	};
+
+};
+
+&clk_hse {
+	clock-frequency = <25000000>;
+};
+
+&usart1 {
+	pinctrl-0 = <&usart1_pins_a>;
+	pinctrl-names = "default";
+	status = "okay";
+};
-- 
1.9.1

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

^ permalink raw reply related

* [PATCHv4 2/2] Bluetooth: add nokia driver
From: Sebastian Reichel @ 2017-04-13  0:26 UTC (permalink / raw)
  To: Sebastian Reichel, Marcel Holtmann, Gustavo Padovan,
	Johan Hedberg, Rob Herring
  Cc: Pavel Machek, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170413002659.25821-1-sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

This adds a driver for the Nokia H4+ protocol, which is used
at least on the Nokia N9, N900 & N950.

Signed-off-by: Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
Changes since PATCHv1:
 * replace __u8 and uint8_t with u8
 * replace __u16 and uint16_t with u16
 * drop BT_BAUDRATE_DIVIDER and use btdev->sysclk_speed * 10 instead
 * fix wording of a sentence
 * fix error path of negotation & alive package receive functions
 * replaced nokia_wait_for_cts with newly introduced serdev function
 * use "nokia,h4p-bluetooth" as compatible string

Changes since PATCHv3:
 * order includes alphabetically
 * explicitly include <linux/of.h>
---
 drivers/bluetooth/Kconfig     |  12 +
 drivers/bluetooth/Makefile    |   2 +
 drivers/bluetooth/hci_nokia.c | 820 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 834 insertions(+)
 create mode 100644 drivers/bluetooth/hci_nokia.c

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index c2c14a12713b..2e3e4d3547ad 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -86,6 +86,18 @@ config BT_HCIUART_H4
 
 	  Say Y here to compile support for HCI UART (H4) protocol.
 
+config BT_HCIUART_NOKIA
+	tristate "UART Nokia H4+ protocol support"
+	depends on BT_HCIUART
+	depends on SERIAL_DEV_BUS
+	depends on PM
+	help
+	  Nokia H4+ is serial protocol for communication between Bluetooth
+	  device and host. This protocol is required for Bluetooth devices
+	  with UART interface in Nokia devices.
+
+	  Say Y here to compile support for Nokia's H4+ protocol.
+
 config BT_HCIUART_BCSP
 	bool "BCSP protocol support"
 	depends on BT_HCIUART
diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index fd571689eed6..a7f237320f4b 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -25,6 +25,8 @@ obj-$(CONFIG_BT_BCM)		+= btbcm.o
 obj-$(CONFIG_BT_RTL)		+= btrtl.o
 obj-$(CONFIG_BT_QCA)		+= btqca.o
 
+obj-$(CONFIG_BT_HCIUART_NOKIA)	+= hci_nokia.o
+
 btmrvl-y			:= btmrvl_main.o
 btmrvl-$(CONFIG_DEBUG_FS)	+= btmrvl_debugfs.o
 
diff --git a/drivers/bluetooth/hci_nokia.c b/drivers/bluetooth/hci_nokia.c
new file mode 100644
index 000000000000..4038daf78d24
--- /dev/null
+++ b/drivers/bluetooth/hci_nokia.c
@@ -0,0 +1,820 @@
+/*
+ *  Bluetooth HCI UART H4 driver with Nokia Extensions AKA Nokia H4+
+ *
+ *  Copyright (C) 2015 Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
+ *  Copyright (C) 2015-2017 Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
+ *
+ *  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.h>
+#include <linux/errno.h>
+#include <linux/firmware.h>
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pm_runtime.h>
+#include <linux/serdev.h>
+#include <linux/skbuff.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <linux/unaligned/le_struct.h>
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+
+#include "hci_uart.h"
+#include "btbcm.h"
+
+#define NOKIA_ID_BCM2048	0x04
+#define NOKIA_ID_TI1271		0x31
+
+#define FIRMWARE_BCM2048	"nokia/bcmfw.bin"
+#define FIRMWARE_TI1271		"nokia/ti1273.bin"
+
+#define HCI_NOKIA_NEG_PKT	0x06
+#define HCI_NOKIA_ALIVE_PKT	0x07
+#define HCI_NOKIA_RADIO_PKT	0x08
+
+#define HCI_NOKIA_NEG_HDR_SIZE		1
+#define HCI_NOKIA_MAX_NEG_SIZE		255
+#define HCI_NOKIA_ALIVE_HDR_SIZE	1
+#define HCI_NOKIA_MAX_ALIVE_SIZE	255
+#define HCI_NOKIA_RADIO_HDR_SIZE	2
+#define HCI_NOKIA_MAX_RADIO_SIZE	255
+
+#define NOKIA_PROTO_PKT		0x44
+#define NOKIA_PROTO_BYTE	0x4c
+
+#define NOKIA_NEG_REQ		0x00
+#define NOKIA_NEG_ACK		0x20
+#define NOKIA_NEG_NAK		0x40
+
+#define H4_TYPE_SIZE		1
+
+#define NOKIA_RECV_ALIVE \
+	.type = HCI_NOKIA_ALIVE_PKT, \
+	.hlen = HCI_NOKIA_ALIVE_HDR_SIZE, \
+	.loff = 0, \
+	.lsize = 1, \
+	.maxlen = HCI_NOKIA_MAX_ALIVE_SIZE \
+
+#define NOKIA_RECV_NEG \
+	.type = HCI_NOKIA_NEG_PKT, \
+	.hlen = HCI_NOKIA_NEG_HDR_SIZE, \
+	.loff = 0, \
+	.lsize = 1, \
+	.maxlen = HCI_NOKIA_MAX_NEG_SIZE \
+
+#define NOKIA_RECV_RADIO \
+	.type = HCI_NOKIA_RADIO_PKT, \
+	.hlen = HCI_NOKIA_RADIO_HDR_SIZE, \
+	.loff = 1, \
+	.lsize = 1, \
+	.maxlen = HCI_NOKIA_MAX_RADIO_SIZE \
+
+struct hci_nokia_neg_hdr {
+	u8	dlen;
+} __packed;
+
+struct hci_nokia_neg_cmd {
+	u8	ack;
+	u16	baud;
+	u16	unused1;
+	u8	proto;
+	u16	sys_clk;
+	u16	unused2;
+} __packed;
+
+#define NOKIA_ALIVE_REQ   0x55
+#define NOKIA_ALIVE_RESP  0xcc
+
+struct hci_nokia_alive_hdr {
+	u8	dlen;
+} __packed;
+
+struct hci_nokia_alive_pkt {
+	u8	mid;
+	u8	unused;
+} __packed;
+
+struct hci_nokia_neg_evt {
+	u8	ack;
+	u16	baud;
+	u16	unused1;
+	u8	proto;
+	u16	sys_clk;
+	u16	unused2;
+	u8	man_id;
+	u8	ver_id;
+} __packed;
+
+#define MAX_BAUD_RATE		3692300
+#define SETUP_BAUD_RATE		921600
+#define INIT_BAUD_RATE		120000
+
+struct hci_nokia_radio_hdr {
+	u8	evt;
+	u8	dlen;
+} __packed;
+
+struct nokia_bt_dev {
+	struct hci_uart hu;
+	struct serdev_device *serdev;
+
+	struct gpio_desc *reset;
+	struct gpio_desc *wakeup_host;
+	struct gpio_desc *wakeup_bt;
+	unsigned long sysclk_speed;
+
+	int wake_irq;
+	struct sk_buff *rx_skb;
+	struct sk_buff_head txq;
+	bdaddr_t bdaddr;
+
+	int init_error;
+	struct completion init_completion;
+
+	u8 man_id;
+	u8 ver_id;
+
+	bool initialized;
+	bool tx_enabled;
+	bool rx_enabled;
+};
+
+static int nokia_enqueue(struct hci_uart *hu, struct sk_buff *skb);
+
+static void nokia_flow_control(struct serdev_device *serdev, bool enable)
+{
+	if (enable) {
+		serdev_device_set_rts(serdev, true);
+		serdev_device_set_flow_control(serdev, true);
+	} else {
+		serdev_device_set_flow_control(serdev, false);
+		serdev_device_set_rts(serdev, false);
+	}
+}
+
+static irqreturn_t wakeup_handler(int irq, void *data)
+{
+	struct nokia_bt_dev *btdev = data;
+	struct device *dev = &btdev->serdev->dev;
+	int wake_state = gpiod_get_value(btdev->wakeup_host);
+
+	if (btdev->rx_enabled == wake_state)
+		return IRQ_HANDLED;
+
+	if (wake_state)
+		pm_runtime_get(dev);
+	else
+		pm_runtime_put(dev);
+
+	btdev->rx_enabled = wake_state;
+
+	return IRQ_HANDLED;
+}
+
+static int nokia_reset(struct hci_uart *hu)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+	int err;
+
+	/* reset routine */
+	gpiod_set_value_cansleep(btdev->reset, 1);
+	gpiod_set_value_cansleep(btdev->wakeup_bt, 1);
+
+	msleep(100);
+
+	/* safety check */
+	err = gpiod_get_value_cansleep(btdev->wakeup_host);
+	if (err == 1) {
+		dev_err(dev, "reset: host wakeup not low!");
+		return -EPROTO;
+	}
+
+	/* flush queue */
+	serdev_device_write_flush(btdev->serdev);
+
+	/* init uart */
+	nokia_flow_control(btdev->serdev, false);
+	serdev_device_set_baudrate(btdev->serdev, INIT_BAUD_RATE);
+
+	gpiod_set_value_cansleep(btdev->reset, 0);
+
+	/* wait for cts */
+	err = serdev_device_wait_for_cts(btdev->serdev, true, 200);
+	if (err < 0) {
+		dev_err(dev, "CTS not received: %d", err);
+		return err;
+	}
+
+	nokia_flow_control(btdev->serdev, true);
+
+	return 0;
+}
+
+static int nokia_send_alive_packet(struct hci_uart *hu)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+	struct hci_nokia_alive_hdr *hdr;
+	struct hci_nokia_alive_pkt *pkt;
+	struct sk_buff *skb;
+	int len;
+
+	init_completion(&btdev->init_completion);
+
+	len = H4_TYPE_SIZE + sizeof(*hdr) + sizeof(*pkt);
+	skb = bt_skb_alloc(len, GFP_KERNEL);
+	if (!skb)
+		return -ENOMEM;
+
+	hci_skb_pkt_type(skb) = HCI_NOKIA_ALIVE_PKT;
+	memset(skb->data, 0x00, len);
+
+	hdr = (struct hci_nokia_alive_hdr *)skb_put(skb, sizeof(*hdr));
+	hdr->dlen = sizeof(*pkt);
+	pkt = (struct hci_nokia_alive_pkt *)skb_put(skb, sizeof(*pkt));
+	pkt->mid = NOKIA_ALIVE_REQ;
+
+	nokia_enqueue(hu, skb);
+	hci_uart_tx_wakeup(hu);
+
+	dev_dbg(dev, "Alive sent");
+
+	if (!wait_for_completion_interruptible_timeout(&btdev->init_completion,
+		msecs_to_jiffies(1000))) {
+		return -ETIMEDOUT;
+	}
+
+	if (btdev->init_error < 0)
+		return btdev->init_error;
+
+	return 0;
+}
+
+static int nokia_send_negotiation(struct hci_uart *hu)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+	struct hci_nokia_neg_cmd *neg_cmd;
+	struct hci_nokia_neg_hdr *neg_hdr;
+	struct sk_buff *skb;
+	int len, err;
+	u16 baud = DIV_ROUND_CLOSEST(btdev->sysclk_speed * 10, SETUP_BAUD_RATE);
+	int sysclk = btdev->sysclk_speed / 1000;
+
+	len = H4_TYPE_SIZE + sizeof(*neg_hdr) + sizeof(*neg_cmd);
+	skb = bt_skb_alloc(len, GFP_KERNEL);
+	if (!skb)
+		return -ENOMEM;
+
+	hci_skb_pkt_type(skb) = HCI_NOKIA_NEG_PKT;
+
+	neg_hdr = (struct hci_nokia_neg_hdr *)skb_put(skb, sizeof(*neg_hdr));
+	neg_hdr->dlen = sizeof(*neg_cmd);
+
+	neg_cmd = (struct hci_nokia_neg_cmd *)skb_put(skb, sizeof(*neg_cmd));
+	neg_cmd->ack = NOKIA_NEG_REQ;
+	neg_cmd->baud = cpu_to_le16(baud);
+	neg_cmd->unused1 = 0x0000;
+	neg_cmd->proto = NOKIA_PROTO_BYTE;
+	neg_cmd->sys_clk = cpu_to_le16(sysclk);
+	neg_cmd->unused2 = 0x0000;
+
+	btdev->init_error = 0;
+	init_completion(&btdev->init_completion);
+
+	nokia_enqueue(hu, skb);
+	hci_uart_tx_wakeup(hu);
+
+	dev_dbg(dev, "Negotiation sent");
+
+	if (!wait_for_completion_interruptible_timeout(&btdev->init_completion,
+		msecs_to_jiffies(10000))) {
+		return -ETIMEDOUT;
+	}
+
+	if (btdev->init_error < 0)
+		return btdev->init_error;
+
+	/* Change to previously negotiated speed. Flow Control
+	 * is disabled until bluetooth adapter is ready to avoid
+	 * broken bytes being received.
+	 */
+	nokia_flow_control(btdev->serdev, false);
+	serdev_device_set_baudrate(btdev->serdev, SETUP_BAUD_RATE);
+	err = serdev_device_wait_for_cts(btdev->serdev, true, 200);
+	if (err < 0) {
+		dev_err(dev, "CTS not received: %d", err);
+		return err;
+	}
+	nokia_flow_control(btdev->serdev, true);
+
+	dev_dbg(dev, "Negotiation successful");
+
+	return 0;
+}
+
+static int nokia_setup_fw(struct hci_uart *hu)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+	const char *fwname;
+	const struct firmware *fw;
+	const u8 *fw_ptr;
+	size_t fw_size;
+	int err;
+
+	dev_dbg(dev, "setup firmware");
+
+	if (btdev->man_id == NOKIA_ID_BCM2048) {
+		fwname = FIRMWARE_BCM2048;
+	} else if (btdev->man_id == NOKIA_ID_TI1271) {
+		fwname = FIRMWARE_TI1271;
+	} else {
+		dev_err(dev, "Unsupported bluetooth device!");
+		return -ENODEV;
+	}
+
+	err = request_firmware(&fw, fwname, dev);
+	if (err < 0) {
+		dev_err(dev, "%s: Failed to load Nokia firmware file (%d)",
+			hu->hdev->name, err);
+		return err;
+	}
+
+	fw_ptr = fw->data;
+	fw_size = fw->size;
+
+	while (fw_size >= 4) {
+		u16 pkt_size = get_unaligned_le16(fw_ptr);
+		u8 pkt_type = fw_ptr[2];
+		const struct hci_command_hdr *cmd;
+		u16 opcode;
+		struct sk_buff *skb;
+
+		switch (pkt_type) {
+		case HCI_COMMAND_PKT:
+			cmd = (struct hci_command_hdr *)(fw_ptr + 3);
+			opcode = le16_to_cpu(cmd->opcode);
+
+			skb = __hci_cmd_sync(hu->hdev, opcode, cmd->plen,
+					     fw_ptr + 3 + HCI_COMMAND_HDR_SIZE,
+					     HCI_INIT_TIMEOUT);
+			if (IS_ERR(skb)) {
+				err = PTR_ERR(skb);
+				dev_err(dev, "%s: FW command %04x failed (%d)",
+				       hu->hdev->name, opcode, err);
+				goto done;
+			}
+			kfree_skb(skb);
+			break;
+		case HCI_NOKIA_RADIO_PKT:
+		case HCI_NOKIA_NEG_PKT:
+		case HCI_NOKIA_ALIVE_PKT:
+			break;
+		}
+
+		fw_ptr += pkt_size + 2;
+		fw_size -= pkt_size + 2;
+	}
+
+done:
+	release_firmware(fw);
+	return err;
+}
+
+static int nokia_setup(struct hci_uart *hu)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+	int err;
+
+	btdev->initialized = false;
+
+	nokia_flow_control(btdev->serdev, false);
+
+	pm_runtime_get_sync(dev);
+
+	if (btdev->tx_enabled) {
+		gpiod_set_value_cansleep(btdev->wakeup_bt, 0);
+		pm_runtime_put(&btdev->serdev->dev);
+		btdev->tx_enabled = false;
+	}
+
+	dev_dbg(dev, "protocol setup");
+
+	/* 0. reset connection */
+	err = nokia_reset(hu);
+	if (err < 0) {
+		dev_err(dev, "Reset failed: %d", err);
+		goto out;
+	}
+
+	/* 1. negotiate speed etc */
+	err = nokia_send_negotiation(hu);
+	if (err < 0) {
+		dev_err(dev, "Negotiation failed: %d", err);
+		goto out;
+	}
+
+	/* 2. verify correct setup using alive packet */
+	err = nokia_send_alive_packet(hu);
+	if (err < 0) {
+		dev_err(dev, "Alive check failed: %d", err);
+		goto out;
+	}
+
+	/* 3. send firmware */
+	err = nokia_setup_fw(hu);
+	if (err < 0) {
+		dev_err(dev, "Could not setup FW: %d", err);
+		goto out;
+	}
+
+	nokia_flow_control(btdev->serdev, false);
+	serdev_device_set_baudrate(btdev->serdev, MAX_BAUD_RATE);
+	nokia_flow_control(btdev->serdev, true);
+
+	if (btdev->man_id == NOKIA_ID_BCM2048) {
+		hu->hdev->set_bdaddr = btbcm_set_bdaddr;
+		set_bit(HCI_QUIRK_INVALID_BDADDR, &hu->hdev->quirks);
+		dev_dbg(dev, "bcm2048 has invalid bluetooth address!");
+	}
+
+	dev_dbg(dev, "protocol setup done!");
+
+	gpiod_set_value_cansleep(btdev->wakeup_bt, 0);
+	pm_runtime_put(dev);
+	btdev->tx_enabled = false;
+	btdev->initialized = true;
+
+	return 0;
+out:
+	pm_runtime_put(dev);
+
+	return err;
+}
+
+static int nokia_open(struct hci_uart *hu)
+{
+	struct device *dev = &hu->serdev->dev;
+
+	dev_dbg(dev, "protocol open");
+
+	serdev_device_open(hu->serdev);
+
+	pm_runtime_enable(dev);
+
+	return 0;
+}
+
+static int nokia_flush(struct hci_uart *hu)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+
+	dev_dbg(&btdev->serdev->dev, "flush device");
+
+	skb_queue_purge(&btdev->txq);
+
+	return 0;
+}
+
+static int nokia_close(struct hci_uart *hu)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+
+	dev_dbg(dev, "close device");
+
+	btdev->initialized = false;
+
+	skb_queue_purge(&btdev->txq);
+
+	kfree_skb(btdev->rx_skb);
+
+	/* disable module */
+	gpiod_set_value(btdev->reset, 1);
+	gpiod_set_value(btdev->wakeup_bt, 0);
+
+	pm_runtime_disable(&btdev->serdev->dev);
+	serdev_device_close(btdev->serdev);
+
+	return 0;
+}
+
+/* Enqueue frame for transmittion (padding, crc, etc) */
+static int nokia_enqueue(struct hci_uart *hu, struct sk_buff *skb)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+	int err;
+
+	/* Prepend skb with frame type */
+	memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
+
+	/* Packets must be word aligned */
+	if (skb->len % 2) {
+		err = skb_pad(skb, 1);
+		if (err)
+			return err;
+		*skb_put(skb, 1) = 0x00;
+	}
+
+	skb_queue_tail(&btdev->txq, skb);
+
+	return 0;
+}
+
+static int nokia_recv_negotiation_packet(struct hci_dev *hdev,
+					 struct sk_buff *skb)
+{
+	struct hci_uart *hu = hci_get_drvdata(hdev);
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+	struct hci_nokia_neg_hdr *hdr;
+	struct hci_nokia_neg_evt *evt;
+	int ret = 0;
+
+	hdr = (struct hci_nokia_neg_hdr *)skb->data;
+	if (hdr->dlen != sizeof(*evt)) {
+		btdev->init_error = -EIO;
+		ret = -EIO;
+		goto finish_neg;
+	}
+
+	evt = (struct hci_nokia_neg_evt *)skb_pull(skb, sizeof(*hdr));
+
+	if (evt->ack != NOKIA_NEG_ACK) {
+		dev_err(dev, "Negotiation received: wrong reply");
+		btdev->init_error = -EINVAL;
+		ret = -EINVAL;
+		goto finish_neg;
+	}
+
+	btdev->man_id = evt->man_id;
+	btdev->ver_id = evt->ver_id;
+
+	dev_dbg(dev, "Negotiation received: baud=%u:clk=%u:manu=%u:vers=%u",
+		evt->baud, evt->sys_clk, evt->man_id, evt->ver_id);
+
+finish_neg:
+	complete(&btdev->init_completion);
+	kfree_skb(skb);
+	return ret;
+}
+
+static int nokia_recv_alive_packet(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct hci_uart *hu = hci_get_drvdata(hdev);
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+	struct hci_nokia_alive_hdr *hdr;
+	struct hci_nokia_alive_pkt *pkt;
+	int ret = 0;
+
+	hdr = (struct hci_nokia_alive_hdr *)skb->data;
+	if (hdr->dlen != sizeof(*pkt)) {
+		dev_err(dev, "Corrupted alive message");
+		btdev->init_error = -EIO;
+		ret = -EIO;
+		goto finish_alive;
+	}
+
+	pkt = (struct hci_nokia_alive_pkt *)skb_pull(skb, sizeof(*hdr));
+
+	if (pkt->mid != NOKIA_ALIVE_RESP) {
+		dev_err(dev, "Alive received: invalid response: 0x%02x!",
+			pkt->mid);
+		btdev->init_error = -EINVAL;
+		ret = -EINVAL;
+		goto finish_alive;
+	}
+
+	dev_dbg(dev, "Alive received");
+
+finish_alive:
+	complete(&btdev->init_completion);
+	kfree_skb(skb);
+	return ret;
+}
+
+static int nokia_recv_radio(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	/* Packets received on the dedicated radio channel are
+	 * HCI events and so feed them back into the core.
+	 */
+	hci_skb_pkt_type(skb) = HCI_EVENT_PKT;
+	return hci_recv_frame(hdev, skb);
+}
+
+/* Recv data */
+static const struct h4_recv_pkt nokia_recv_pkts[] = {
+	{ H4_RECV_ACL,		.recv = hci_recv_frame },
+	{ H4_RECV_SCO,		.recv = hci_recv_frame },
+	{ H4_RECV_EVENT,	.recv = hci_recv_frame },
+	{ NOKIA_RECV_ALIVE,	.recv = nokia_recv_alive_packet },
+	{ NOKIA_RECV_NEG,	.recv = nokia_recv_negotiation_packet },
+	{ NOKIA_RECV_RADIO,	.recv = nokia_recv_radio },
+};
+
+static int nokia_recv(struct hci_uart *hu, const void *data, int count)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+	int err;
+
+	if (!test_bit(HCI_UART_REGISTERED, &hu->flags))
+		return -EUNATCH;
+
+	btdev->rx_skb = h4_recv_buf(hu->hdev, btdev->rx_skb, data, count,
+				  nokia_recv_pkts, ARRAY_SIZE(nokia_recv_pkts));
+	if (IS_ERR(btdev->rx_skb)) {
+		err = PTR_ERR(btdev->rx_skb);
+		dev_err(dev, "Frame reassembly failed (%d)", err);
+		btdev->rx_skb = NULL;
+		return err;
+	}
+
+	return count;
+}
+
+static struct sk_buff *nokia_dequeue(struct hci_uart *hu)
+{
+	struct nokia_bt_dev *btdev = hu->priv;
+	struct device *dev = &btdev->serdev->dev;
+	struct sk_buff *result = skb_dequeue(&btdev->txq);
+
+	if (!btdev->initialized)
+		return result;
+
+	if (btdev->tx_enabled == !!result)
+		return result;
+
+	if (result) {
+		pm_runtime_get_sync(dev);
+		gpiod_set_value_cansleep(btdev->wakeup_bt, 1);
+	} else {
+		serdev_device_wait_until_sent(btdev->serdev, 0);
+		gpiod_set_value_cansleep(btdev->wakeup_bt, 0);
+		pm_runtime_put(dev);
+	}
+
+	btdev->tx_enabled = !!result;
+
+	return result;
+}
+
+static const struct hci_uart_proto nokia_proto = {
+	.id		= HCI_UART_NOKIA,
+	.name		= "Nokia",
+	.open		= nokia_open,
+	.close		= nokia_close,
+	.recv		= nokia_recv,
+	.enqueue	= nokia_enqueue,
+	.dequeue	= nokia_dequeue,
+	.flush		= nokia_flush,
+	.setup		= nokia_setup,
+	.manufacturer	= 1,
+};
+
+static int nokia_bluetooth_serdev_probe(struct serdev_device *serdev)
+{
+	struct device *dev = &serdev->dev;
+	struct nokia_bt_dev *btdev;
+	struct clk *sysclk;
+	int err = 0;
+
+	btdev = devm_kzalloc(dev, sizeof(*btdev), GFP_KERNEL);
+	if (!btdev)
+		return -ENOMEM;
+
+	btdev->hu.serdev = btdev->serdev = serdev;
+	serdev_device_set_drvdata(serdev, btdev);
+
+	btdev->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(btdev->reset)) {
+		err = PTR_ERR(btdev->reset);
+		dev_err(dev, "could not get reset gpio: %d", err);
+		return err;
+	}
+
+	btdev->wakeup_host = devm_gpiod_get(dev, "host-wakeup", GPIOD_IN);
+	if (IS_ERR(btdev->wakeup_host)) {
+		err = PTR_ERR(btdev->wakeup_host);
+		dev_err(dev, "could not get host wakeup gpio: %d", err);
+		return err;
+	}
+
+	btdev->wake_irq = gpiod_to_irq(btdev->wakeup_host);
+
+	err = devm_request_threaded_irq(dev, btdev->wake_irq, NULL,
+		wakeup_handler,
+		IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+		"wakeup", btdev);
+	if (err) {
+		dev_err(dev, "could request wakeup irq: %d", err);
+		return err;
+	}
+
+	btdev->wakeup_bt = devm_gpiod_get(dev, "bluetooth-wakeup",
+					   GPIOD_OUT_LOW);
+	if (IS_ERR(btdev->wakeup_bt)) {
+		err = PTR_ERR(btdev->wakeup_bt);
+		dev_err(dev, "could not get BT wakeup gpio: %d", err);
+		return err;
+	}
+
+	sysclk = devm_clk_get(dev, "sysclk");
+	if (IS_ERR(sysclk)) {
+		err = PTR_ERR(sysclk);
+		dev_err(dev, "could not get sysclk: %d", err);
+		return err;
+	}
+
+	clk_prepare_enable(sysclk);
+	btdev->sysclk_speed = clk_get_rate(sysclk);
+	clk_disable_unprepare(sysclk);
+
+	skb_queue_head_init(&btdev->txq);
+
+	btdev->hu.priv = btdev;
+	btdev->hu.alignment = 2; /* Nokia H4+ is word aligned */
+
+	err = hci_uart_register_device(&btdev->hu, &nokia_proto);
+	if (err) {
+		dev_err(dev, "could not register bluetooth uart: %d", err);
+		return err;
+	}
+
+	return 0;
+}
+
+static void nokia_bluetooth_serdev_remove(struct serdev_device *serdev)
+{
+	struct nokia_bt_dev *btdev = serdev_device_get_drvdata(serdev);
+	struct hci_uart *hu = &btdev->hu;
+	struct hci_dev *hdev = hu->hdev;
+
+	cancel_work_sync(&hu->write_work);
+
+	hci_unregister_dev(hdev);
+	hci_free_dev(hdev);
+	hu->proto->close(hu);
+
+	pm_runtime_disable(&btdev->serdev->dev);
+}
+
+static int nokia_bluetooth_runtime_suspend(struct device *dev)
+{
+	struct serdev_device *serdev = to_serdev_device(dev);
+
+	nokia_flow_control(serdev, false);
+	return 0;
+}
+
+static int nokia_bluetooth_runtime_resume(struct device *dev)
+{
+	struct serdev_device *serdev = to_serdev_device(dev);
+
+	nokia_flow_control(serdev, true);
+	return 0;
+}
+
+static const struct dev_pm_ops nokia_bluetooth_pm_ops = {
+	SET_RUNTIME_PM_OPS(nokia_bluetooth_runtime_suspend,
+			   nokia_bluetooth_runtime_resume,
+			   NULL)
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id nokia_bluetooth_of_match[] = {
+	{ .compatible = "nokia,h4p-bluetooth", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, nokia_bluetooth_of_match);
+#endif
+
+static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
+	.probe = nokia_bluetooth_serdev_probe,
+	.remove = nokia_bluetooth_serdev_remove,
+	.driver = {
+		.name = "nokia-bluetooth",
+		.pm = &nokia_bluetooth_pm_ops,
+		.of_match_table = of_match_ptr(nokia_bluetooth_of_match),
+	},
+};
+
+module_serdev_device_driver(nokia_bluetooth_serdev_driver);
-- 
2.11.0

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

^ permalink raw reply related

* [PATCHv4 1/2] dt-bindings: net: bluetooth: Add nokia-bluetooth
From: Sebastian Reichel @ 2017-04-13  0:26 UTC (permalink / raw)
  To: Sebastian Reichel, Marcel Holtmann, Gustavo Padovan,
	Johan Hedberg, Rob Herring
  Cc: Pavel Machek, linux-bluetooth, devicetree, linux-kernel
In-Reply-To: <20170413002659.25821-1-sre@kernel.org>

Add binding document for serial bluetooth chips using
Nokia H4+ protocol.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
Changes since PATCHv1:
 * change compatible strings
 * mention active high/low state for GPIOs
---
 .../devicetree/bindings/net/nokia-bluetooth.txt    | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/nokia-bluetooth.txt

diff --git a/Documentation/devicetree/bindings/net/nokia-bluetooth.txt b/Documentation/devicetree/bindings/net/nokia-bluetooth.txt
new file mode 100644
index 000000000000..42be7dc9a70b
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/nokia-bluetooth.txt
@@ -0,0 +1,51 @@
+Nokia Bluetooth Chips
+---------------------
+
+Nokia phones often come with UART connected bluetooth chips from different
+vendors and modified device API. Those devices speak a protocol named H4+
+(also known as h4p) by Nokia, which is similar to the H4 protocol from the
+Bluetooth standard. In addition to the H4 protocol it specifies two more
+UART status lines for wakeup of UART transceivers to improve power management
+and a few new packet types used to negotiate uart speed.
+
+Required properties:
+
+ - compatible: should contain "nokia,h4p-bluetooth" as well as one of the following:
+   * "brcm,bcm2048-nokia"
+   * "ti,wl1271-bluetooth-nokia"
+ - reset-gpios: GPIO specifier, used to reset the BT module (active low)
+ - bluetooth-wakeup-gpios: GPIO specifier, used to wakeup the BT module (active high)
+ - host-wakeup-gpios: GPIO specifier, used to wakeup the host processor (active high)
+ - clock-names: should be "sysclk"
+ - clocks: should contain a clock specifier for every name in clock-names
+
+Optional properties:
+
+ - None
+
+Example:
+
+/ {
+       /* controlled (enabled/disabled) directly by BT module */
+       bluetooth_clk: vctcxo {
+               compatible = "fixed-clock";
+               #clock-cells = <0>;
+               clock-frequency = <38400000>;
+       };
+};
+
+&uart2 {
+       pinctrl-names = "default";
+       pinctrl-0 = <&uart2_pins>;
+
+       bluetooth {
+               compatible = "ti,wl1271-bluetooth-nokia", "nokia,h4p-bluetooth";
+
+               reset-gpios = <&gpio1 26 GPIO_ACTIVE_LOW>; /* gpio26 */
+               host-wakeup-gpios = <&gpio4 5 GPIO_ACTIVE_HIGH>; /* gpio101 */
+               bluetooth-wakeup-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>; /* gpio37 */
+
+               clocks = <&bluetooth_clk>;
+               clock-names = "sysclk";
+       };
+};
-- 
2.11.0

^ permalink raw reply related

* [PATCHv4 0/2] Nokia H4+ support
From: Sebastian Reichel @ 2017-04-13  0:26 UTC (permalink / raw)
  To: Sebastian Reichel, Marcel Holtmann, Gustavo Padovan,
	Johan Hedberg, Rob Herring
  Cc: Pavel Machek, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Hi,

Here is the fourth revision of the nokia bluetooth patchset. Compared
to the previous one the following things have changed:

 * Drop patch 1-8 (applied by Marcel) and update Cc/To for patchset
 * Order includes in the driver alphabetically
 * Explicitly include <linux/of.h> in the nokia bluetooth driver

-- Sebastian

Sebastian Reichel (2):
  dt-bindings: net: bluetooth: Add nokia-bluetooth
  Bluetooth: add nokia driver

 .../devicetree/bindings/net/nokia-bluetooth.txt    |  51 ++
 drivers/bluetooth/Kconfig                          |  12 +
 drivers/bluetooth/Makefile                         |   2 +
 drivers/bluetooth/hci_nokia.c                      | 820 +++++++++++++++++++++
 4 files changed, 885 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/nokia-bluetooth.txt
 create mode 100644 drivers/bluetooth/hci_nokia.c

-- 
2.11.0

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

^ permalink raw reply

* Re: [PATCHv3 00/10] Nokia H4+ support
From: Sebastian Reichel @ 2017-04-13  0:26 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Greg Kroah-Hartman, Gustavo F. Padovan, Johan Hedberg,
	Samuel Thibault, Pavel Machek, Tony Lindgren, Jiri Slaby,
	Mark Rutland, open list:BLUETOOTH DRIVERS,
	linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	David S. Miller, Rob Herring
In-Reply-To: <C16140C4-0264-411B-9058-0B9013AAC82F-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 3591 bytes --]

Hi Marcel,

On Wed, Apr 12, 2017 at 10:19:21PM +0200, Marcel Holtmann wrote:
> Hi Sebastian,
> 
> >>>>>>>> Here is PATCHv3 for the Nokia bluetooth patchset. I addressed all comments from
> >>>>>>>> Rob and Pavel regarding the serdev patches and dropped the *.dts patches, since
> >>>>>>>> they were queued by Tony. I also changed the patch order, so that the serdev
> >>>>>>>> patches come first. All of them have Acked-by from Rob, so I think it makes
> >>>>>>>> sense to merge them to serdev subsystem (now) and provide an immutable branch
> >>>>>>>> for the bluetooth subsystem.
> >>>>>>> 
> >>>>>>> Greg doesn't read cover letters generally and since the serdev patches
> >>>>>>> are Cc rather than To him, he's probably not planning to pick them up.
> >>>>>> 
> >>>>>> I wonder actually if we should merge all of these via bluetooth-next
> >>>>>> tree with proper Ack from Greg. However it would be good to also get
> >>>>>> buy in from Dave for merging this ultimately through net-next.
> >>>>> 
> >>>>> I don't really care where it goes.  I can take the whole thing in my
> >>>>> tty/serial tree now if no one objects and I get an ack from the relevant
> >>>>> maintainers {hint...}
> >>>> 
> >>>> I think it is better if it goes thru BT tree. I have another driver
> >>>> converted that is dependent on this series. There's a couple other
> >>>> serdev changes on the list too, but this shouldn't depend on them.
> >>> 
> >>> Is this waiting for something, or could it be queued to
> >>> bluetooth-next then? It would be nice to finally have
> >>> this in 4.12 :)
> >> 
> >> I would prefer if we can get an ACK from Greg. Then I merge it through the bluetooth-next tree.
> > 
> > Sorry thought this was coming through mine:
> > 	Acked-by: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> > 
> > Merge away!
> 
> so I have applied patches 1-8 to bluetooth-next tree.
> 
> The last 2 I left out since they do cause build issues on non-DT
> platforms. We need to be able to build the driver on all platforms
> so that sanity compile checks happen all the time.
> 
>   CC      drivers/bluetooth/hci_nokia.o
> drivers/bluetooth/hci_nokia.c:802:34: error: array type has incomplete element type ‘struct of_device_id’
>  static const struct of_device_id nokia_bluetooth_of_match[] = {
>                                   ^~~~~~~~~~~~~~~~~~~~~~~~
> drivers/bluetooth/hci_nokia.c:803:4: error: field name not in record or union initializer
>   { .compatible = "nokia,h4p-bluetooth", },
>     ^
> drivers/bluetooth/hci_nokia.c:803:4: note: (near initialization for ‘nokia_bluetooth_of_match’)
> drivers/bluetooth/hci_nokia.c:815:21: error: implicit declaration of function ‘of_match_ptr’ [-Werror=implicit-function-declaration]
>    .of_match_table = of_match_ptr(nokia_bluetooth_of_match),
>                      ^~~~~~~~~~~~
> drivers/bluetooth/hci_nokia.c:802:34: warning: ‘nokia_bluetooth_of_match’ defined but not used [-Wunused-variable]
>  static const struct of_device_id nokia_bluetooth_of_match[] = {
>                                   ^~~~~~~~~~~~~~~~~~~~~~~~

Building without CONFIG_OF should work already. Note, that its
actually enabled in your build, since nokia_bluetooth_of_match
is guarded by "#ifdef CONFIG_OF". The actual problem is, that
<linux/of.h> is not included in your build. Looks like it was
implicitly included in my configurations, so I didn't notice.
I will send PATCHv4 with the added include and includes sorted
alphabetically.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* RE: [PATCH] ARM: dts: stm32f7: add STM32f769I & stm32f746 discovery board support
From: Vikas MANOCHA @ 2017-04-13  0:03 UTC (permalink / raw)
  To: Alexandre TORGUE, Patrice CHOTARD
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM PORT, open list, Mark Rutland, Maxime Coquelin,
	Rob Herring, Russell King
In-Reply-To: <e6592982-9c24-bfde-469b-770218bf88be-qxv4g6HH51o@public.gmane.org>

Hi Alex,

> -----Original Message-----
> From: Alexandre TORGUE
> Sent: Tuesday, April 11, 2017 12:51 AM
> To: Vikas MANOCHA <vikas.manocha-qxv4g6HH51o@public.gmane.org>; Patrice CHOTARD <patrice.chotard-qxv4g6HH51o@public.gmane.org>
> Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>; moderated list:ARM PORT
> <linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>; open list <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>; Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>; Maxime
> Coquelin <mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>; Russell King <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
> Subject: Re: [PATCH] ARM: dts: stm32f7: add STM32f769I & stm32f746 discovery board support
> 
> Hi Vikas
> 
> On 04/10/2017 08:40 PM, Vikas Manocha wrote:
> > Thanks Alex,
> >
> > On 04/10/2017 12:23 AM, Alexandre Torgue wrote:
> >> Hi
> >>
> >> On 04/08/2017 03:12 AM, Vikas Manocha wrote:
> >>> Stm32f769I & stm32f746 are MCUs of stm32f7 family. Here are the
> >>> major spces of the two boards:
> >>>
> >>> stm32f769I discovery board:
> >>>     - Cortex-M7 core @216MHz
> >>>     - 2MB mcu internal flash
> >>>     - 512KB internal sram
> >>>     - 16MB sdram memory
> >>>     - 64MB qspi flash memory
> >>>     - 4 inch wvga LCD-TFT Display
> >>>
> >>> stm32f746 discovery board:
> >>>     - Cortex-M7 core @216MHz
> >>>     - 1MB mcu internal flash
> >>>     - 320KB internal sram
> >>>     - 8MB sdram memory
> >>>     - 16MB qspi flash memory
> >>>     - 4.3 inch 480x272 LCD-TFT display
> >>>
> >>> Signed-off-by: Vikas Manocha <vikas.manocha-qxv4g6HH51o@public.gmane.org>
> >>> ---
> >>>  arch/arm/boot/dts/Makefile            |   2 +
> >>>  arch/arm/boot/dts/stm32f746-disco.dts | 101 ++++++++++++++++++++++++++++++++++
> >>>  arch/arm/boot/dts/stm32f746.dtsi      |   2 +-
> >>>  arch/arm/boot/dts/stm32f769-disco.dts | 101
> >>> ++++++++++++++++++++++++++++++++++
> >>>  4 files changed, 205 insertions(+), 1 deletion(-)  create mode
> >>> 100644 arch/arm/boot/dts/stm32f746-disco.dts
> >>>  create mode 100644 arch/arm/boot/dts/stm32f769-disco.dts
> >>>
> >>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> >>> index 0118084..a119f74 100644
> >>> --- a/arch/arm/boot/dts/Makefile
> >>> +++ b/arch/arm/boot/dts/Makefile
> >>> @@ -763,6 +763,8 @@ dtb-$(CONFIG_ARCH_STI) += \
> >>> dtb-$(CONFIG_ARCH_STM32)+= \
> >>>      stm32f429-disco.dtb \
> >>>      stm32f469-disco.dtb \
> >>> +    stm32f746-disco.dtb \
> >>> +    stm32f769-disco.dtb \
> >>>      stm32429i-eval.dtb \
> >>>      stm32746g-eval.dtb
> >>>  dtb-$(CONFIG_MACH_SUN4I) += \
> >>> diff --git a/arch/arm/boot/dts/stm32f746-disco.dts
> >>> b/arch/arm/boot/dts/stm32f746-disco.dts
> >>> new file mode 100644
> >>> index 0000000..c0e313f
> >>> --- /dev/null
> >>> +++ b/arch/arm/boot/dts/stm32f746-disco.dts
> >>> @@ -0,0 +1,101 @@
> >>> +/*
> >>> + * Copyright 2017 - Vikas MANOCHA <vikas.manocha-qxv4g6HH51o@public.gmane.org>
> >>> + *
> >>> + * This file is dual-licensed: you can use it either under the
> >>> +terms
> >>> + * of the GPL or the X11 license, at your option. Note that this
> >>> +dual
> >>> + * licensing only applies to this file, and not this project as a
> >>> + * whole.
> >>> + *
> >>> + *  a) This file 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 file 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.
> >>> + *
> >>> + * Or, alternatively,
> >>> + *
> >>> + *  b) Permission is hereby granted, free of charge, to any person
> >>> + *     obtaining a copy of this software and associated documentation
> >>> + *     files (the "Software"), to deal in the Software without
> >>> + *     restriction, including without limitation the rights to use,
> >>> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> >>> + *     sell copies of the Software, and to permit persons to whom the
> >>> + *     Software is furnished to do so, subject to the following
> >>> + *     conditions:
> >>> + *
> >>> + *     The above copyright notice and this permission notice shall be
> >>> + *     included in all copies or substantial portions of the Software.
> >>> + *
> >>> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> >>> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> >>> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> >>> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> >>> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> >>> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> >>> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> >>> + *     OTHER DEALINGS IN THE SOFTWARE.
> >>> + */
> >>> +
> >>> +/dts-v1/;
> >>> +#include "stm32f746.dtsi"
> >>> +#include <dt-bindings/input/input.h>
> >>> +
> >>> +/ {
> >>> +    model = "STMicroelectronics STM32F746-DISCO board";
> >>> +    compatible = "st,stm32f746-disco", "st,stm32f746";
> >>> +
> >>> +    chosen {
> >>> +        bootargs = "root=/dev/ram";
> >>> +        stdout-path = "serial0:115200n8";
> >>> +    };
> >>> +
> >>> +    memory {
> >>> +        reg = <0xC0000000 0x800000>;
> >>> +    };
> >>> +
> >>> +    aliases {
> >>> +        serial0 = &usart1;
> >>> +    };
> >>> +
> >>> +};
> >>> +
> >>> +&clk_hse {
> >>> +    clock-frequency = <25000000>;
> >>> +};
> >>> +
> >>> +&pinctrl {
> >>
> >>
> >> Pin muxing is not defined in board file. Please move it into SOC dtsi file.

Ok, I will move pin muxing in the stm32f746.dtsi file as per current implementation & send V2.

> >
> > Pin muxing used is different for different boards. e.g. usart1_rx pad is PA10 for stm32f769-disco board while it is PB7 for stm32f746-
> disco board.
> > The other possibilities for same pad (usart1_rx) is PB15. To make situation bit more complex, it is only available in f769 device.
> >
> > Putting in SOC dtsi file means having lot of combinations for different pins in separate groups.
> > e.g. only for one instance of one ip (usart1), following groups might be required at one point of time:
> >
> > usart1_pa10_pa9 {..}
> > usart1_pa10_pb14 {..}
> > usart1_pa10_pb6 {..}
> >
> > usart1_pb7_pa9 {..}
> > usart1_pb7_pb14 {..}
> > usart1_pb7_pb6 {..}
> >
> > usart1_pb15_pa9 {..}
> > usart1_pb15_pb14 {..}
> > usart1_pb15_pb6 {..}
> >
> > In case of boards based on stm32f746 device, all the above mentioned groups with pb14 & pb15 will not be available.
> > One solution (to avoid using not available groups) could be to have separate dtsi (or separate pinmux.dtsi) for different devices of
> same family like one for stm32f746 & other for stm32f769. Still it does not resolve the need to have lot of groups combinations for
> each instance of every peripheral in dtsi as mentioned above.
> 
> Yes, it is what I want to have. I did the job on for STM32F4 (F429 / F469). You could have a look on ARM Linux patchwork:
> https://patchwork.kernel.org/patch/9669433/
> 
> To sum-up the implementation:
> 
> -Pinmuxing is defined in separate files which will be included in board dts file.
> 
> -We have a common pinmuxing file + a dedicated pinmuxing file per SOC.
> 
> Example for STMF469-disco:
> stm32f4-pinctrl.dtsi --> stm32f469-pinctrl.dtsi --> stm32f469-disco.dts
> 
> stm32f4-pinctrl.dtsi contains common pinmuxing bindings between STM32F4 stm32f469-pinctrl.dtsi contains dedicated pinmuxing
> bindings for
> STM32F469 (ex: QSPI pins, gpio-ranges ...)
> 
> This implementation is under review.

Sounds good.

Cheers,
Vikas

> 
> 
> > It seems cleaner solution would be pin muxing in board dts file. Please let me know if there is some drawback of this approach. One
> point which i can think of is : duplication of pinmux groups in different board dts files.
> 
> Pinmuxing in board file is currently not my choice. In the board file we only select the group to use.
> 
> Regards
> Alex
> 
> 
> >
> > Cheers,
> > Vikas
> >
> >>> +    usart1_pins: usart1@0    {
> >>> +        pins1 {
> >>> +            pinmux = <STM32F746_PA9_FUNC_USART1_TX>;
> >>> +                bias-disable;
> >>> +                drive-push-pull;
> >>> +                slew-rate = <2>;
> >>> +        };
> >>> +        pins2 {
> >>> +            pinmux = <STM32F746_PB7_FUNC_USART1_RX>;
> >>> +            bias-disable;
> >>> +        };
> >>> +    };
> >>> +
> >>> +    qspi_pins: qspi@0 {
> >>> +        pins {
> >>> +            pinmux = <STM32F746_PB2_FUNC_QUADSPI_CLK>,
> >>> +                   <STM32F746_PB6_FUNC_QUADSPI_BK1_NCS>,
> >>> +                   <STM32F746_PD11_FUNC_QUADSPI_BK1_IO0>,
> >>> +                   <STM32F746_PD12_FUNC_QUADSPI_BK1_IO1>,
> >>> +                   <STM32F746_PD13_FUNC_QUADSPI_BK1_IO3>,
> >>> +                   <STM32F746_PE2_FUNC_QUADSPI_BK1_IO2>;
> >>> +            slew-rate = <2>;
> >>> +        };
> >>> +    };
> >>> +};
> >>> +
> >>> +&usart1 {
> >>> +    pinctrl-0 = <&usart1_pins>;
> >>> +    pinctrl-names = "default";
> >>> +    status = "okay";
> >>> +};
> >>> diff --git a/arch/arm/boot/dts/stm32f746.dtsi
> >>> b/arch/arm/boot/dts/stm32f746.dtsi
> >>> index f321ffe..826700f 100644
> >>> --- a/arch/arm/boot/dts/stm32f746.dtsi
> >>> +++ b/arch/arm/boot/dts/stm32f746.dtsi
> >>> @@ -178,7 +178,7 @@
> >>>              interrupts = <1>, <2>, <3>, <6>, <7>, <8>, <9>, <10>, <23>, <40>, <41>, <42>, <62>, <76>;
> >>>          };
> >>>
> >>> -        pin-controller {
> >>> +        pinctrl: pin-controller {
> >>>              #address-cells = <1>;
> >>>              #size-cells = <1>;
> >>>              compatible = "st,stm32f746-pinctrl"; diff --git
> >>> a/arch/arm/boot/dts/stm32f769-disco.dts
> >>> b/arch/arm/boot/dts/stm32f769-disco.dts
> >>> new file mode 100644
> >>> index 0000000..5f8558e
> >>> --- /dev/null
> >>> +++ b/arch/arm/boot/dts/stm32f769-disco.dts
> >>> @@ -0,0 +1,101 @@
> >>> +/*
> >>> + * Copyright 2017 - Vikas MANOCHA <vikas.manocha-qxv4g6HH51o@public.gmane.org>
> >>> + *
> >>> + * This file is dual-licensed: you can use it either under the
> >>> +terms
> >>> + * of the GPL or the X11 license, at your option. Note that this
> >>> +dual
> >>> + * licensing only applies to this file, and not this project as a
> >>> + * whole.
> >>> + *
> >>> + *  a) This file 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 file 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.
> >>> + *
> >>> + * Or, alternatively,
> >>> + *
> >>> + *  b) Permission is hereby granted, free of charge, to any person
> >>> + *     obtaining a copy of this software and associated documentation
> >>> + *     files (the "Software"), to deal in the Software without
> >>> + *     restriction, including without limitation the rights to use,
> >>> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> >>> + *     sell copies of the Software, and to permit persons to whom the
> >>> + *     Software is furnished to do so, subject to the following
> >>> + *     conditions:
> >>> + *
> >>> + *     The above copyright notice and this permission notice shall be
> >>> + *     included in all copies or substantial portions of the Software.
> >>> + *
> >>> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> >>> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> >>> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> >>> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> >>> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> >>> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> >>> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> >>> + *     OTHER DEALINGS IN THE SOFTWARE.
> >>> + */
> >>> +
> >>> +/dts-v1/;
> >>> +#include "stm32f746.dtsi"
> >>> +#include <dt-bindings/input/input.h>
> >>> +
> >>> +/ {
> >>> +    model = "STMicroelectronics STM32F769-DISCO board";
> >>> +    compatible = "st,stm32f769-disco", "st,stm32f7";
> >>> +
> >>> +    chosen {
> >>> +        bootargs = "root=/dev/ram";
> >>> +        stdout-path = "serial0:115200n8";
> >>> +    };
> >>> +
> >>> +    memory {
> >>> +        reg = <0xC0000000 0x1000000>;
> >>> +    };
> >>> +
> >>> +    aliases {
> >>> +        serial0 = &usart1;
> >>> +    };
> >>> +
> >>> +};
> >>> +
> >>> +&clk_hse {
> >>> +    clock-frequency = <25000000>;
> >>> +};
> >>> +
> >>> +&pinctrl {
> >>
> >> same.
> >>
> >>> +    usart1_pins: usart1@0    {
> >>> +        pins1 {
> >>> +            pinmux = <STM32F746_PA9_FUNC_USART1_TX>;
> >>> +                bias-disable;
> >>> +                drive-push-pull;
> >>> +                slew-rate = <2>;
> >>> +        };
> >>> +        pins2 {
> >>> +            pinmux = <STM32F746_PA10_FUNC_USART1_RX>;
> >>> +            bias-disable;
> >>> +        };
> >>> +    };
> >>> +
> >>> +    qspi_pins: qspi@0 {
> >>> +        pins {
> >>> +            pinmux = <STM32F746_PB2_FUNC_QUADSPI_CLK>,
> >>> +                   <STM32F746_PB6_FUNC_QUADSPI_BK1_NCS>,
> >>> +                   <STM32F746_PC9_FUNC_QUADSPI_BK1_IO0>,
> >>> +                   <STM32F746_PC10_FUNC_QUADSPI_BK1_IO1>,
> >>> +                   <STM32F746_PD13_FUNC_QUADSPI_BK1_IO3>,
> >>> +                   <STM32F746_PE2_FUNC_QUADSPI_BK1_IO2>;
> >>> +            slew-rate = <2>;
> >>> +        };
> >>> +    };
> >>> +};
> >>> +
> >>> +&usart1 {
> >>> +    pinctrl-0 = <&usart1_pins>;
> >>> +    pinctrl-names = "default";
> >>> +    status = "okay";
> >>> +};
> >>>
> >> .
> >>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC net-next] of: mdio: Honor hints from MDIO bus drivers
From: Florian Fainelli @ 2017-04-12 23:58 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	Rob Herring, Frank Rowand,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE, open list
In-Reply-To: <20170412221016.GA29708-g2DYL2Zd6BY@public.gmane.org>

On 04/12/2017 03:10 PM, Andrew Lunn wrote:
>>>>> To give some more background and rational for this change.
>>>>>
>>>>> On a platform where we have a parent MDIO bus, backed by the
>>>>> mdio-bcm-unimac.c driver, we also register a slave MII bus (through
>>>>> net/dsa/dsa2.c) which is parented to this UniMAC MDIO bus through an
>>>>> assignment of of_node. This slave MII bus is created in order to
>>>>> intercept reads/writes to problematic addresses (e.g: that clashes with
>>>>> another piece of hardware).
>>>>>
>>>>> This means that the slave DSA MII bus inherits all child nodes from the
>>>>> originating master MII bus. This also means that when the slave MII bus
>>>>> is probed via of_mdiobus_register(), we probe the same devices twice:
>>>>> once through the master, another time through the slave.
>>>>
>>>> Ah, O.K. This makes more sense. On the hardware i have, we get three
>>>> deep in MDIO busses. We have the FEC mdio bus. On top of that we have
>>>> a gpio-mux-mdio, and on top of that we have the mv88e6xxx mdio
>>>> bus. And i've never seen issues.
>>>>
>>>> So your real problem here is you have two mdio busses using the same
>>>> device tree properties. I would actually say that is just plain
>>>> broken.
>>>
>>> From a Device Tree/HW representation perspective, we do have the
>>> external BCM53125 switch physically attached to the 7445/7278
>>> SWITCH_MDIO bus (backed by mdio-bcm-unimac) so in that regard the
>>> representation is correct. There is also an integrated Gigabit PHY
>>> (bcm7xxx) which is attached to that bus.
> 
> This is made harder by you talking about a board which does not appear
> to have its DT file in mainline. So i'm having to guess what it looks
> like.

The DT binding is in tree and provides an example of how the switch
looks like, below is the example, but I am also adding the MDIO bus and
the PHYs just so you can see how things wind up:

switch_top@f0b00000 {
        compatible = "simple-bus";
        #size-cells = <1>;
        #address-cells = <1>;
        ranges = <0 0xf0b00000 0x40804>;

        ethernet_switch@0 {
                compatible = "brcm,bcm7445-switch-v4.0";
                #size-cells = <0>;
                #address-cells = <1>;
                reg = <0x0 0x40000
                        0x40000 0x110
                        0x40340 0x30
                        0x40380 0x30
                        0x40400 0x34
                        0x40600 0x208>;
                reg-names = "core", "reg", intrl2_0", "intrl2_1",
                            "fcb, "acb";
                interrupts = <0 0x18 0
                                0 0x19 0>;
                brcm,num-gphy = <1>;
                brcm,num-rgmii-ports = <2>;
                brcm,fcb-pause-override;
                brcm,acb-packets-inflight;

                ports {
                        #address-cells = <1>;
                        #size-cells = <0>;

                        port@0 {
                                label = "gphy";
                                reg = <0>;
				phy-handle = <&phy5>;
                        };

			sw0port1: port@1 {
				label = "rgmii_1";
				reg = <1>;
				phy-mode = "rgmii";
				fixed-link {
					speed = <1000>;
					full-duplex;
				};
			}
                };
        };

	mdio@403c0 {
		reg = <0x403c0 0x8 0x40300 0x18>;
		#address-cells = <0x1>;
		#size-cells = <0x0>;
		compatible = "brcm,unimac-mdio";
		reg-names = "mdio", "mdio_indir_rw";

		switch: switch@0 {
			broken-turn-around;
			reg = <0x0>;
			compatible = "brcm,bcm53125";
			#address-cells = <1>;
			#size-cells = <0>;

			ports {
				..
				port@8 {
					ethernet = <&sw0port1>;
				};
				...
			};
		};

		phy5: ethernet-phy@5 {
			reg = <0x5>;
			compatible = "ethernet-phy-ieee802.3-c22";
		};
	};
};


> 
> So what i think we are talking about is this bit of code:
> 
> static int bcm_sf2_mdio_register(struct dsa_switch *ds)
> {
>         struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
>         struct device_node *dn;
>         static int index;
>         int err;
> 
>         /* Find our integrated MDIO bus node */
>         dn = of_find_compatible_node(NULL, NULL, "brcm,unimac-mdio");
>         priv->master_mii_bus = of_mdio_find_bus(dn);
>         if (!priv->master_mii_bus)
>                 return -EPROBE_DEFER;
> 
>         get_device(&priv->master_mii_bus->dev);
>         priv->master_mii_dn = dn;
> 
>         priv->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
>         if (!priv->slave_mii_bus)
>                 return -ENOMEM;
> 
>         priv->slave_mii_bus->priv = priv;
>         priv->slave_mii_bus->name = "sf2 slave mii";
>         priv->slave_mii_bus->read = bcm_sf2_sw_mdio_read;
>         priv->slave_mii_bus->write = bcm_sf2_sw_mdio_write;
>         snprintf(priv->slave_mii_bus->id, MII_BUS_ID_SIZE, "sf2-%d",
>                  index++);
>         priv->slave_mii_bus->dev.of_node = dn;
> 
> If i get you right, your switch is hanging off the MDIO bus
> "brcm,unimac-mdio" you find the dn for. You then register another MDIO
> bus using the exact same node? How does that make any sense? Isn't it
> a physical separate MDIO bus?

First, the main switch is memory mapped into the 7445 SoC's register
space. This switch has an external MDIO bus which connects to an
integrated Gigabit PHY at MDIO address 5, but also to a BCM53125 switch
at address 30.

Because of a bug in the D0 revision of the 7445, programming the
BCM53125 switch through MDIO ends-up programming the 7445 memory mapped
switch as well because the integrated 7445 switch has its pseudo-PHY
snooping accesses to the MDIO bus! What was done to work-around this is
to create a slave MII bus through DSA, and divert the reads/writes
from/to the BCM53125 by instead using internal 7445 switch registers
which isolate its pseudo-PHY from the MDIO bus, thus no double
programming anymore.

Since the BCM53125 switch is a) physically attached to the mdio@403c0
node, and b) needs to have visibility in the OF world for DSA to probe
it, this is what I did here.

The slave MII bus is using the same node here because that's the
simplest way to make this bus inherit the devices of interest from the
parent bus.

> So it should have its own set of nodes
> in the device tree. This is how we do it for the Marvell switches. See
> Documentation/devicetree/binding/net/dsa/marvell.txt and
> arch/arm/boot/dts/vf610-zii-dev-rev-b.dts. That DT blob uses
> phy-handle to link the switch ports to the phys on the mdio bus.

>From a pure HW representation, this is not quite correct, because the
switch is physically attached to mdio@403c0, but since we are
pathologically broken, we need something different here...
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] ARM: dts: stm32f7: add stm32f769I & stm32f746 discovery board support
From: Vikas Manocha @ 2017-04-12 23:50 UTC (permalink / raw)
  To: alexandre.torgue-qxv4g6HH51o, patrice.chotard-qxv4g6HH51o
  Cc: Vikas Manocha,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM PORT, open list, Mark Rutland, Maxime Coquelin,
	Rob Herring, Russell King

Stm32f769I & stm32f746 are MCUs of stm32f7 family. Here are the major
specs of the two boards:

stm32f769I discovery board:
	- Cortex-M7 core @216MHz
	- 2MB mcu internal flash
	- 512KB internal sram
	- 16MB sdram memory
	- 64MB qspi flash memory
	- 4 inch wvga LCD-TFT Display

stm32f746 discovery board:
	- Cortex-M7 core @216MHz
	- 1MB mcu internal flash
	- 320KB internal sram
	- 8MB sdram memory
	- 16MB qspi flash memory
	- 4.3 inch 480x272 LCD-TFT display

Signed-off-by: Vikas Manocha <vikas.manocha-qxv4g6HH51o@public.gmane.org>
---

Changed in v2:
- moved pin muxing from board dts file to dtsi file.

 arch/arm/boot/dts/Makefile            |  2 +
 arch/arm/boot/dts/stm32f746-disco.dts | 74 +++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/stm32f746.dtsi      | 12 ++++++
 arch/arm/boot/dts/stm32f769-disco.dts | 74 +++++++++++++++++++++++++++++++++++
 4 files changed, 162 insertions(+)
 create mode 100644 arch/arm/boot/dts/stm32f746-disco.dts
 create mode 100644 arch/arm/boot/dts/stm32f769-disco.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 0118084..a119f74 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -763,6 +763,8 @@ dtb-$(CONFIG_ARCH_STI) += \
 dtb-$(CONFIG_ARCH_STM32)+= \
 	stm32f429-disco.dtb \
 	stm32f469-disco.dtb \
+	stm32f746-disco.dtb \
+	stm32f769-disco.dtb \
 	stm32429i-eval.dtb \
 	stm32746g-eval.dtb
 dtb-$(CONFIG_MACH_SUN4I) += \
diff --git a/arch/arm/boot/dts/stm32f746-disco.dts b/arch/arm/boot/dts/stm32f746-disco.dts
new file mode 100644
index 0000000..55afae9
--- /dev/null
+++ b/arch/arm/boot/dts/stm32f746-disco.dts
@@ -0,0 +1,74 @@
+/*
+ * Copyright 2017 - Vikas MANOCHA <vikas.manocha-qxv4g6HH51o@public.gmane.org>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file 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 file 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.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "stm32f746.dtsi"
+#include <dt-bindings/input/input.h>
+
+/ {
+	model = "STMicroelectronics STM32F746-DISCO board";
+	compatible = "st,stm32f746-disco", "st,stm32f746";
+
+	chosen {
+		bootargs = "root=/dev/ram";
+		stdout-path = "serial0:115200n8";
+	};
+
+	memory {
+		reg = <0xC0000000 0x800000>;
+	};
+
+	aliases {
+		serial0 = &usart1;
+	};
+
+};
+
+&clk_hse {
+	clock-frequency = <25000000>;
+};
+
+&usart1 {
+	pinctrl-0 = <&usart1_pins_pa9_pa10>;
+	pinctrl-names = "default";
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/stm32f746.dtsi b/arch/arm/boot/dts/stm32f746.dtsi
index f321ffe..1064fb1 100644
--- a/arch/arm/boot/dts/stm32f746.dtsi
+++ b/arch/arm/boot/dts/stm32f746.dtsi
@@ -287,6 +287,18 @@
 					bias-disable;
 				};
 			};
+			usart1_pins_pa9_pa10: usart1@1 {
+				pins1 {
+					pinmux = <STM32F746_PA9_FUNC_USART1_TX>;
+					bias-disable;
+					drive-push-pull;
+					slew-rate = <0>;
+				};
+				pins2 {
+					pinmux = <STM32F746_PA10_FUNC_USART1_RX>;
+					bias-disable;
+				};
+			};
 		};
 
 		rcc: rcc@40023800 {
diff --git a/arch/arm/boot/dts/stm32f769-disco.dts b/arch/arm/boot/dts/stm32f769-disco.dts
new file mode 100644
index 0000000..166728a
--- /dev/null
+++ b/arch/arm/boot/dts/stm32f769-disco.dts
@@ -0,0 +1,74 @@
+/*
+ * Copyright 2017 - Vikas MANOCHA <vikas.manocha-qxv4g6HH51o@public.gmane.org>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file 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 file 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.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "stm32f746.dtsi"
+#include <dt-bindings/input/input.h>
+
+/ {
+	model = "STMicroelectronics STM32F769-DISCO board";
+	compatible = "st,stm32f769-disco", "st,stm32f7";
+
+	chosen {
+		bootargs = "root=/dev/ram";
+		stdout-path = "serial0:115200n8";
+	};
+
+	memory {
+		reg = <0xC0000000 0x1000000>;
+	};
+
+	aliases {
+		serial0 = &usart1;
+	};
+
+};
+
+&clk_hse {
+	clock-frequency = <25000000>;
+};
+
+&usart1 {
+	pinctrl-0 = <&usart1_pins_a>;
+	pinctrl-names = "default";
+	status = "okay";
+};
-- 
1.9.1

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

^ permalink raw reply related

* Re: [RFC net-next] of: mdio: Honor hints from MDIO bus drivers
From: Andrew Lunn @ 2017-04-12 22:10 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	Rob Herring, Frank Rowand,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE, open list
In-Reply-To: <417fdaa1-7aba-e646-8a50-043322f1410d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

> >>> To give some more background and rational for this change.
> >>>
> >>> On a platform where we have a parent MDIO bus, backed by the
> >>> mdio-bcm-unimac.c driver, we also register a slave MII bus (through
> >>> net/dsa/dsa2.c) which is parented to this UniMAC MDIO bus through an
> >>> assignment of of_node. This slave MII bus is created in order to
> >>> intercept reads/writes to problematic addresses (e.g: that clashes with
> >>> another piece of hardware).
> >>>
> >>> This means that the slave DSA MII bus inherits all child nodes from the
> >>> originating master MII bus. This also means that when the slave MII bus
> >>> is probed via of_mdiobus_register(), we probe the same devices twice:
> >>> once through the master, another time through the slave.
> >>
> >> Ah, O.K. This makes more sense. On the hardware i have, we get three
> >> deep in MDIO busses. We have the FEC mdio bus. On top of that we have
> >> a gpio-mux-mdio, and on top of that we have the mv88e6xxx mdio
> >> bus. And i've never seen issues.
> >>
> >> So your real problem here is you have two mdio busses using the same
> >> device tree properties. I would actually say that is just plain
> >> broken.
> > 
> > From a Device Tree/HW representation perspective, we do have the
> > external BCM53125 switch physically attached to the 7445/7278
> > SWITCH_MDIO bus (backed by mdio-bcm-unimac) so in that regard the
> > representation is correct. There is also an integrated Gigabit PHY
> > (bcm7xxx) which is attached to that bus.

This is made harder by you talking about a board which does not appear
to have its DT file in mainline. So i'm having to guess what it looks
like.

So what i think we are talking about is this bit of code:

static int bcm_sf2_mdio_register(struct dsa_switch *ds)
{
        struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
        struct device_node *dn;
        static int index;
        int err;

        /* Find our integrated MDIO bus node */
        dn = of_find_compatible_node(NULL, NULL, "brcm,unimac-mdio");
        priv->master_mii_bus = of_mdio_find_bus(dn);
        if (!priv->master_mii_bus)
                return -EPROBE_DEFER;

        get_device(&priv->master_mii_bus->dev);
        priv->master_mii_dn = dn;

        priv->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
        if (!priv->slave_mii_bus)
                return -ENOMEM;

        priv->slave_mii_bus->priv = priv;
        priv->slave_mii_bus->name = "sf2 slave mii";
        priv->slave_mii_bus->read = bcm_sf2_sw_mdio_read;
        priv->slave_mii_bus->write = bcm_sf2_sw_mdio_write;
        snprintf(priv->slave_mii_bus->id, MII_BUS_ID_SIZE, "sf2-%d",
                 index++);
        priv->slave_mii_bus->dev.of_node = dn;

If i get you right, your switch is hanging off the MDIO bus
"brcm,unimac-mdio" you find the dn for. You then register another MDIO
bus using the exact same node? How does that make any sense? Isn't it
a physical separate MDIO bus? So it should have its own set of nodes
in the device tree. This is how we do it for the Marvell switches. See
Documentation/devicetree/binding/net/dsa/marvell.txt and
arch/arm/boot/dts/vf610-zii-dev-rev-b.dts. That DT blob uses
phy-handle to link the switch ports to the phys on the mdio bus.

	   Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC net-next] of: mdio: Honor hints from MDIO bus drivers
From: Florian Fainelli @ 2017-04-12 21:48 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	Rob Herring, Frank Rowand,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE, open list
In-Reply-To: <6de72d4c-5634-f3d6-5bfd-fcc0acda0b83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On 04/11/2017 04:23 PM, Florian Fainelli wrote:
> On 04/11/2017 04:14 PM, Andrew Lunn wrote:
>>> To give some more background and rational for this change.
>>>
>>> On a platform where we have a parent MDIO bus, backed by the
>>> mdio-bcm-unimac.c driver, we also register a slave MII bus (through
>>> net/dsa/dsa2.c) which is parented to this UniMAC MDIO bus through an
>>> assignment of of_node. This slave MII bus is created in order to
>>> intercept reads/writes to problematic addresses (e.g: that clashes with
>>> another piece of hardware).
>>>
>>> This means that the slave DSA MII bus inherits all child nodes from the
>>> originating master MII bus. This also means that when the slave MII bus
>>> is probed via of_mdiobus_register(), we probe the same devices twice:
>>> once through the master, another time through the slave.
>>
>> Ah, O.K. This makes more sense. On the hardware i have, we get three
>> deep in MDIO busses. We have the FEC mdio bus. On top of that we have
>> a gpio-mux-mdio, and on top of that we have the mv88e6xxx mdio
>> bus. And i've never seen issues.
>>
>> So your real problem here is you have two mdio busses using the same
>> device tree properties. I would actually say that is just plain
>> broken.
> 
> From a Device Tree/HW representation perspective, we do have the
> external BCM53125 switch physically attached to the 7445/7278
> SWITCH_MDIO bus (backed by mdio-bcm-unimac) so in that regard the
> representation is correct. There is also an integrated Gigabit PHY
> (bcm7xxx) which is attached to that bus.
> 
> From a SW perspective though, we want to talk to the integrated Gigabit
> PHY using mdio-bcm-unimac but talk to the external BCM53125 switch using
> the slave MII bus created by the bcm_sf2 driver in order to create an
> isolation. We need to inherit some of the parent (mdio-bcm-unimac) child
> DT nodes (such as the BCM53125), but not the GPHY. The easiest solution
> I found was to use this patch.
> 
> Using mdiobus_register() instead of of_mdiobus_register() was
> considered, but then, the child BCM53125 has no more "visbility" into
> the OF world at all, and it matters, because this switch is also driven
> via a DSA switch driver and its Ethernet data-path is connected to one
> port of the bcm_sf2 switch..
> 
> Thankfully the HW bug was fixed eventually ;)

In fact, all I need is to flag the internal Gigabit PHY for the slave
MII bus node with something that makes it appear as "disabled" which I
can presumably do with of_update_property() and putting a status =
"disabled" property in there. Let me do something like that and see how
big of a hack this becomes.
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v1 1/1] mtd: mtk-nor: set controller's address width according to nor flash
From: Cyrille Pitchen @ 2017-04-12 20:57 UTC (permalink / raw)
  To: Guochun Mao, Cyrille Pitchen
  Cc: Mark Rutland, devicetree, Richard Weinberger, Russell King,
	linux-kernel, Rob Herring, linux-mtd, Matthias Brugger,
	linux-mediatek, David Woodhouse, linux-arm-kernel
In-Reply-To: <1491381462-21893-2-git-send-email-guochun.mao@mediatek.com>

Hi Guochun,

Le 05/04/2017 à 10:37, Guochun Mao a écrit :
> When nor's size larger than 16MByte, nor's address width maybe
> set to 3 or 4, and controller should change address width according
> to nor's setting.
> 
> Signed-off-by: Guochun Mao <guochun.mao@mediatek.com>
> ---
>  drivers/mtd/spi-nor/mtk-quadspi.c |   27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c
> index e661877..b637770 100644
> --- a/drivers/mtd/spi-nor/mtk-quadspi.c
> +++ b/drivers/mtd/spi-nor/mtk-quadspi.c
> @@ -104,6 +104,8 @@
>  #define MTK_NOR_MAX_RX_TX_SHIFT		6
>  /* can shift up to 56 bits (7 bytes) transfer by MTK_NOR_PRG_CMD */
>  #define MTK_NOR_MAX_SHIFT		7
> +/* nor controller 4-byte address mode enable bit */
> +#define MTK_NOR_4B_ADDR_EN		BIT(4)
>  
>  /* Helpers for accessing the program data / shift data registers */
>  #define MTK_NOR_PRG_REG(n)		(MTK_NOR_PRGDATA0_REG + 4 * (n))
> @@ -230,10 +232,35 @@ static int mt8173_nor_write_buffer_disable(struct mt8173_nor *mt8173_nor)
>  				  10000);
>  }
>  
> +static void mt8173_nor_set_addr_width(struct mt8173_nor *mt8173_nor)
> +{
> +	u8 val;
> +	struct spi_nor *nor = &mt8173_nor->nor;
> +
> +	val = readb(mt8173_nor->base + MTK_NOR_DUAL_REG);
> +
> +	switch (nor->addr_width) {
> +	case 3:
> +		val &= ~MTK_NOR_4B_ADDR_EN;
> +		break;
> +	case 4:
> +		val |= MTK_NOR_4B_ADDR_EN;
> +		break;
> +	default:
> +		dev_warn(mt8173_nor->dev, "Unexpected address width %u.\n",
> +			 nor->addr_width);
> +		break;
> +	}
> +
> +	writeb(val, mt8173_nor->base + MTK_NOR_DUAL_REG);
> +}
> +
>  static void mt8173_nor_set_addr(struct mt8173_nor *mt8173_nor, u32 addr)
>  {
>  	int i;
>  
> +	mt8173_nor_set_addr_width(mt8173_nor);
> +
>  	for (i = 0; i < 3; i++) {

Should it be 'i < nor->addr_width' instead of 'i < 3' ?
Does it work when accessing data after 128Mbit ?

Best regards,

Cyrille

>  		writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR0_REG + i * 4);
>  		addr >>= 8;
> 

^ permalink raw reply

* Re: [PATCH v2 2/3] net: macb: Remove CONFIG_OF around DT match table
From: Rob Herring @ 2017-04-12 20:51 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-kernel@vger.kernel.org, Andrew Lunn, Vivien Didelot,
	Lee Jones, Nicolas Ferre, Frank Rowand,
	open list:NETWORKING DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE
In-Reply-To: <20170412044156.17351-3-f.fainelli@gmail.com>

On Tue, Apr 11, 2017 at 11:41 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> A subsequent patch is going to make of_match_node() an inline stub when
> CONFIG_OF is disabled, which will let the compiler eliminate unused variables.
> In order not to clutter the code more, remove the CONFIG_OF #ifdef such that
> macb_dt_ids and what it references are always defined.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  drivers/net/ethernet/cadence/macb.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 30606b11b128..01016e9525ee 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -2811,7 +2811,6 @@ static int macb_init(struct platform_device *pdev)
>         return 0;
>  }
>
> -#if defined(CONFIG_OF)
>  /* 1518 rounded up */
>  #define AT91ETHER_MAX_RBUFF_SZ 0x600
>  /* max number of receive buffers */
> @@ -3215,7 +3214,6 @@ static const struct of_device_id macb_dt_ids[] = {
>         { /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, macb_dt_ids);

Did you actually check that the compiler can eliminate things when
modules are enabled? Because it's not going to be able to eliminate
MODULE_DEVICE_TABLE entry AFAICT.

Rob

^ permalink raw reply

* Re: [PATCH v3 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller
From: Cyrille Pitchen @ 2017-04-12 20:28 UTC (permalink / raw)
  To: Ludovic Barre, Cyrille Pitchen, Marek Vasut
  Cc: Boris Brezillon, Alexandre Torgue,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Richard Weinberger,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Brian Norris,
	David Woodhouse
In-Reply-To: <1492016785-24976-3-git-send-email-ludovic.Barre-qxv4g6HH51o@public.gmane.org>

Hi Ludovic,

Globally I will say this is pretty good and, IMHO, almost ready to be
merged into the github/spi-nor tree.

I have few comments, all but one are mainly recommendations hence not
blocking on my side. However one should be fixed. Even if not critical
and very unlikely to happen, I think it is more a potential bug than a
simple cosmetic issue.

So if you have time to prepare a new series, I will try if possible to
add your patch into the PR, which should be sent ideally before the end
of the week. I can't give you any guarantee since the timing is really
short now. Besides Marek still needs to add his Acked-by / Reviewed-by
once satisfied, otherwise your patch will have to wait for the next release.

If something I say is wrong or if you disagree with me, do not hesitate:
we can discuss and I can change my mind.

So my comments to follow:

Le 12/04/2017 à 19:06, Ludovic Barre a écrit :
> From: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
> 
> The quadspi is a specialized communication interface targeting single,
> dual or quad SPI Flash memories.
> 
> It can operate in any of the following modes:
> -indirect mode: all the operations are performed using the quadspi
>  registers
> -read memory-mapped mode: the external Flash memory is mapped to the
>  microcontroller address space and is seen by the system as if it was
>  an internal memory
> 
> Signed-off-by: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
> ---
>  drivers/mtd/spi-nor/Kconfig         |   7 +
>  drivers/mtd/spi-nor/Makefile        |   1 +
>  drivers/mtd/spi-nor/stm32-quadspi.c | 693 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 701 insertions(+)
>  create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c
> 
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 7252087..bfdfb1e 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -106,4 +106,11 @@ config SPI_INTEL_SPI_PLATFORM
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called intel-spi-platform.
>  
> +config SPI_STM32_QUADSPI
> +	tristate "STM32 Quad SPI controller"
> +	depends on ARCH_STM32
> +	help
> +	  This enables support for the STM32 Quad SPI controller.
> +	  We only connect the NOR to this controller.
> +
>  endif # MTD_SPI_NOR
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index 72238a7..285aab8 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_MTD_MT81xx_NOR)    += mtk-quadspi.o
>  obj-$(CONFIG_SPI_NXP_SPIFI)	+= nxp-spifi.o
>  obj-$(CONFIG_SPI_INTEL_SPI)	+= intel-spi.o
>  obj-$(CONFIG_SPI_INTEL_SPI_PLATFORM)	+= intel-spi-platform.o
> +obj-$(CONFIG_SPI_STM32_QUADSPI)	+= stm32-quadspi.o
> \ No newline at end of file
> diff --git a/drivers/mtd/spi-nor/stm32-quadspi.c b/drivers/mtd/spi-nor/stm32-quadspi.c
> new file mode 100644
> index 0000000..9e90dee
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/stm32-quadspi.c
> @@ -0,0 +1,693 @@
> +/*
> + * stm32_quadspi.c
> + *
> + * Copyright (C) 2017, Ludovic Barre
> + *
> + * License terms: GNU General Public License (GPL), version 2
> + */
> +#include <linux/clk.h>
> +#include <linux/errno.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/partitions.h>
> +#include <linux/mtd/spi-nor.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +
> +#define QUADSPI_CR		0x00
> +#define CR_EN			BIT(0)
> +#define CR_ABORT		BIT(1)
> +#define CR_DMAEN		BIT(2)
> +#define CR_TCEN			BIT(3)
> +#define CR_SSHIFT		BIT(4)
> +#define CR_DFM			BIT(6)
> +#define CR_FSEL			BIT(7)
> +#define CR_FTHRES_SHIFT		8
> +#define CR_FTHRES_MASK		GENMASK(12, 8)
> +#define CR_FTHRES(n)		(((n) << CR_FTHRES_SHIFT) & CR_FTHRES_MASK)
> +#define CR_TEIE			BIT(16)
> +#define CR_TCIE			BIT(17)
> +#define CR_FTIE			BIT(18)
> +#define CR_SMIE			BIT(19)
> +#define CR_TOIE			BIT(20)
> +#define CR_PRESC_SHIFT		24
> +#define CR_PRESC_MASK		GENMASK(31, 24)
> +#define CR_PRESC(n)		(((n) << CR_PRESC_SHIFT) & CR_PRESC_MASK)
> +
> +#define QUADSPI_DCR		0x04
> +#define DCR_CSHT_SHIFT		8
> +#define DCR_CSHT_MASK		GENMASK(10, 8)
> +#define DCR_CSHT(n)		(((n) << DCR_CSHT_SHIFT) & DCR_CSHT_MASK)
> +#define DCR_FSIZE_SHIFT		16
> +#define DCR_FSIZE_MASK		GENMASK(20, 16)
> +#define DCR_FSIZE(n)		(((n) << DCR_FSIZE_SHIFT) & DCR_FSIZE_MASK)
> +
> +#define QUADSPI_SR		0x08
> +#define SR_TEF			BIT(0)
> +#define SR_TCF			BIT(1)
> +#define SR_FTF			BIT(2)
> +#define SR_SMF			BIT(3)
> +#define SR_TOF			BIT(4)
> +#define SR_BUSY			BIT(5)
> +#define SR_FLEVEL_SHIFT		8
> +#define SR_FLEVEL_MASK		GENMASK(13, 8)
> +
> +#define QUADSPI_FCR		0x0c
> +#define FCR_CTCF		BIT(1)
> +
> +#define QUADSPI_DLR		0x10
> +
> +#define QUADSPI_CCR		0x14
> +#define CCR_INST_SHIFT		0
> +#define CCR_INST_MASK		GENMASK(7, 0)
> +#define CCR_INST(n)		(((n) << CCR_INST_SHIFT) & CCR_INST_MASK)
> +#define CCR_IMODE_NONE		(0 << 8)
> +#define CCR_IMODE_1		(1 << 8)
> +#define CCR_IMODE_2		(2 << 8)
> +#define CCR_IMODE_4		(3 << 8)
> +#define CCR_ADMODE_NONE		(0 << 10)
> +#define CCR_ADMODE_1		(1 << 10)
> +#define CCR_ADMODE_2		(2 << 10)
> +#define CCR_ADMODE_4		(3 << 10)

For bitfields, it would have been better to use (1u << 8) that is
unsigned int instead of signed int. It might avoid some warnings or even
bugs depending on how you use those macros, especially if you were using
BIT(31). The GENMASK() macro generates an unsigned int on its side.

However, I guess you have tested your driver so if you have noticed any
issue, I will say this is not blocking for me :)

You don't use BIT(31) and I don't see any use of the >> operator so I
think it should be safe. My comment is just a recommendation for the
next time!

To be fair, you can point out the atmel_qspi.c driver and I did the same
mistake but like you, I didn't use the >> operator with the bitmask I
have defined :p

> +#define CCR_ADSIZE_SHIFT	12
> +#define CCR_ADSIZE_MASK		GENMASK(13, 12)
> +#define CCR_ADSIZE(n)		(((n) << CCR_ADSIZE_SHIFT) & CCR_ADSIZE_MASK)
> +#define CCR_ABMODE_NONE		(0 << 14)
> +#define CCR_ABMODE_1		(1 << 14)
> +#define CCR_ABMODE_2		(2 << 14)
> +#define CCR_ABMODE_4		(3 << 14)
> +#define CCR_ABSIZE_8		(0 << 16)
> +#define CCR_ABSIZE_16		(1 << 16)
> +#define CCR_ABSIZE_24		(2 << 16)
> +#define CCR_ABSIZE_32		(3 << 16)
> +#define CCR_DCYC_SHIFT		18
> +#define CCR_DCYC_MASK		GENMASK(22, 18)
> +#define CCR_DCYC(n)		(((n) << CCR_DCYC_SHIFT) & CCR_DCYC_MASK)
> +#define CCR_DMODE_NONE		(0 << 24)
> +#define CCR_DMODE_1		(1 << 24)
> +#define CCR_DMODE_2		(2 << 24)
> +#define CCR_DMODE_4		(3 << 24)
> +#define CCR_FMODE_INDW		(0 << 26)
> +#define CCR_FMODE_INDR		(1 << 26)
> +#define CCR_FMODE_APM		(2 << 26)
> +#define CCR_FMODE_MM		(3 << 26)
> +
> +#define QUADSPI_AR		0x18
> +#define QUADSPI_ABR		0x1c
> +#define QUADSPI_DR		0x20
> +#define QUADSPI_PSMKR		0x24
> +#define QUADSPI_PSMAR		0x28
> +#define QUADSPI_PIR		0x2c
> +#define QUADSPI_LPTR		0x30
> +#define LPTR_DFT_TIMEOUT	0x10
> +
> +#define FSIZE_VAL(size)		(__fls(size) - 1)
> +
> +#define STM32_MAX_MMAP_SZ	SZ_256M
> +#define STM32_MAX_NORCHIP	2
> +
> +#define STM32_QSPI_FIFO_TIMEOUT_US 30000
> +#define STM32_QSPI_BUSY_TIMEOUT_US 100000
> +
> +struct stm32_qspi_flash {
> +	struct spi_nor nor;
> +	u32 cs;
> +	u32 fsize;
> +	u32 presc;
> +	u32 read_mode;
> +	struct stm32_qspi *qspi;
> +};
> +
> +struct stm32_qspi {
> +	struct device *dev;
> +	void __iomem *io_base;
> +	void __iomem *mm_base;
> +	resource_size_t mm_size;
> +	u32 nor_num;
> +	struct clk *clk;
> +	u32 clk_rate;
> +	struct stm32_qspi_flash flash[STM32_MAX_NORCHIP];
> +	struct completion cmd_completion;
> +
> +	/*
> +	 * to protect device configuration, could be different between
> +	 * 2 flash access (bk1, bk2)
> +	 */
> +	struct mutex lock;
> +};
> +
> +struct stm32_qspi_cmd {
> +	u8 addr_width;
> +	u8 dummy;
> +	bool tx_data;
> +	u8 opcode;
> +	u32 framemode;
> +	u32 qspimode;
> +	u32 addr;
> +	size_t len;
> +	void *buf;
> +};
> +
> +static int stm32_qspi_wait_cmd(struct stm32_qspi *qspi)
> +{
> +	u32 cr;
> +	int err = 0;
> +
> +	if (readl_relaxed(qspi->io_base + QUADSPI_SR) & SR_TCF)
> +		return 0;
> +
> +	reinit_completion(&qspi->cmd_completion);
> +	cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
> +	writel_relaxed(cr | CR_TCIE, qspi->io_base + QUADSPI_CR);
> +
> +	if (!wait_for_completion_interruptible_timeout(&qspi->cmd_completion,
> +						       msecs_to_jiffies(1000)))
> +		err = -ETIMEDOUT;
> +
> +	writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
> +	return err;
> +}
> +
> +static int stm32_qspi_wait_nobusy(struct stm32_qspi *qspi)
> +{
> +	u32 sr;
> +
> +	return readl_relaxed_poll_timeout(qspi->io_base + QUADSPI_SR, sr,
> +					  !(sr & SR_BUSY), 10,
> +					  STM32_QSPI_BUSY_TIMEOUT_US);
> +}
> +
> +static void stm32_qspi_set_framemode(struct spi_nor *nor,
> +				     struct stm32_qspi_cmd *cmd, bool read)
> +{
> +	u32 dmode = CCR_DMODE_1;
> +
> +	cmd->framemode = CCR_IMODE_1;
> +
> +	if (read) {
> +		switch (nor->flash_read) {
> +		case SPI_NOR_NORMAL:
> +		case SPI_NOR_FAST:
> +			dmode = CCR_DMODE_1;
> +			break;
> +		case SPI_NOR_DUAL:
> +			dmode = CCR_DMODE_2;
> +			break;
> +		case SPI_NOR_QUAD:
> +			dmode = CCR_DMODE_4;
> +			break;
> +		}
> +	}
> +
> +	cmd->framemode |= cmd->tx_data ? dmode : 0;
> +	cmd->framemode |= cmd->addr_width ? CCR_ADMODE_1 : 0;
> +}
> +
> +static void stm32_qspi_read_fifo(u8 *val, void __iomem *addr)
> +{
> +	*val = readb_relaxed(addr);
> +}
> +
> +static void stm32_qspi_write_fifo(u8 *val, void __iomem *addr)
> +{
> +	writeb_relaxed(*val, addr);
> +}
> +
> +static int stm32_qspi_tx_poll(struct stm32_qspi *qspi,
> +			      const struct stm32_qspi_cmd *cmd)
> +{
> +	void (*tx_fifo)(u8 *, void __iomem *);
> +	u32 len = cmd->len, sr;
> +	u8 *buf = cmd->buf;
> +	int ret;
> +
> +	if (cmd->qspimode == CCR_FMODE_INDW)
> +		tx_fifo = stm32_qspi_write_fifo;
> +	else
> +		tx_fifo = stm32_qspi_read_fifo;
> +
> +	while (len--) {
> +		ret = readl_relaxed_poll_timeout(qspi->io_base + QUADSPI_SR,
> +						 sr, (sr & SR_FTF), 10,
> +						 STM32_QSPI_FIFO_TIMEOUT_US);
> +		if (ret) {
> +			dev_err(qspi->dev, "fifo timeout (stat:%#x)\n", sr);
> +			break;
> +		}
> +		tx_fifo(buf++, qspi->io_base + QUADSPI_DR);
> +	}
> +
> +	return ret;
> +}
> +
> +static int stm32_qspi_tx_mm(struct stm32_qspi *qspi,
> +			    const struct stm32_qspi_cmd *cmd)
> +{
> +	memcpy_fromio(cmd->buf, qspi->mm_base + cmd->addr, cmd->len);
> +	return 0;
> +}
> +
> +static int stm32_qspi_tx(struct stm32_qspi *qspi,
> +			 const struct stm32_qspi_cmd *cmd)
> +{
> +	if (!cmd->tx_data)
> +		return 0;
> +
> +	if (cmd->qspimode == CCR_FMODE_MM)
> +		return stm32_qspi_tx_mm(qspi, cmd);
> +
> +	return stm32_qspi_tx_poll(qspi, cmd);
> +}
> +
> +static int stm32_qspi_send(struct stm32_qspi_flash *flash,
> +			   const struct stm32_qspi_cmd *cmd)
> +{
> +	struct stm32_qspi *qspi = flash->qspi;
> +	u32 ccr, dcr, cr;
> +	int err;
> +
> +	err = stm32_qspi_wait_nobusy(qspi);
> +	if (err)
> +		goto abort;
> +
> +	dcr = readl_relaxed(qspi->io_base + QUADSPI_DCR) & ~DCR_FSIZE_MASK;
> +	dcr |= DCR_FSIZE(flash->fsize);
> +	writel_relaxed(dcr, qspi->io_base + QUADSPI_DCR);
> +
> +	cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
> +	cr &= ~CR_PRESC_MASK & ~CR_FSEL;
> +	cr |= CR_PRESC(flash->presc);
> +	cr |= flash->cs ? CR_FSEL : 0;
> +	writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
> +
> +	if (cmd->tx_data)
> +		writel_relaxed(cmd->len - 1, qspi->io_base + QUADSPI_DLR);
> +
> +	ccr = cmd->framemode | cmd->qspimode;
> +
> +	if (cmd->dummy)
> +		ccr |= CCR_DCYC(cmd->dummy);
> +
> +	if (cmd->addr_width)
> +		ccr |= CCR_ADSIZE(cmd->addr_width - 1);
> +
> +	ccr |= CCR_INST(cmd->opcode);
> +	writel_relaxed(ccr, qspi->io_base + QUADSPI_CCR);
> +
> +	if (cmd->addr_width && cmd->qspimode != CCR_FMODE_MM)
> +		writel_relaxed(cmd->addr, qspi->io_base + QUADSPI_AR);
> +
> +	err = stm32_qspi_tx(qspi, cmd);
> +	if (err)
> +		goto abort;
> +
> +	if (cmd->qspimode != CCR_FMODE_MM) {
> +		err = stm32_qspi_wait_cmd(qspi);
> +		if (err)
> +			goto abort;
> +		writel_relaxed(FCR_CTCF, qspi->io_base + QUADSPI_FCR);
> +	}
> +
> +	return err;
> +
> +abort:
> +	cr = readl_relaxed(qspi->io_base + QUADSPI_CR) | CR_ABORT;
> +	writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
> +
> +	dev_err(qspi->dev, "%s abort err:%d\n", __func__, err);
> +	return err;
> +}
> +
> +static int stm32_qspi_read_reg(struct spi_nor *nor,
> +			       u8 opcode, u8 *buf, int len)
> +{
> +	struct stm32_qspi_flash *flash = nor->priv;
> +	struct device *dev = flash->qspi->dev;
> +	struct stm32_qspi_cmd cmd;
> +
> +	dev_dbg(dev, "read_reg: cmd:%#.2x buf:%p len:%#x\n", opcode, buf, len);
> +
> +	memset(&cmd, 0, sizeof(cmd));
> +	cmd.opcode = opcode;
> +	cmd.tx_data = true;
> +	cmd.len = len;
> +	cmd.buf = buf;
> +	cmd.qspimode = CCR_FMODE_INDR;
> +
> +	stm32_qspi_set_framemode(nor, &cmd, false);
> +
> +	return stm32_qspi_send(flash, &cmd);
> +}
> +
> +static int stm32_qspi_write_reg(struct spi_nor *nor, u8 opcode,
> +				u8 *buf, int len)
> +{
> +	struct stm32_qspi_flash *flash = nor->priv;
> +	struct device *dev = flash->qspi->dev;
> +	struct stm32_qspi_cmd cmd;
> +
> +	dev_dbg(dev, "write_reg: cmd:%#.2x buf:%p len:%#x\n", opcode, buf, len);
> +
> +	memset(&cmd, 0, sizeof(cmd));
> +	cmd.opcode = opcode;
> +	cmd.tx_data = !!(buf && len > 0);
> +	cmd.len = len;
> +	cmd.buf = buf;
> +	cmd.qspimode = CCR_FMODE_INDW;
> +
> +	stm32_qspi_set_framemode(nor, &cmd, false);
> +
> +	return stm32_qspi_send(flash, &cmd);
> +}
> +
> +static ssize_t stm32_qspi_read(struct spi_nor *nor, loff_t from, size_t len,
> +			       u_char *buf)
> +{
> +	struct stm32_qspi_flash *flash = nor->priv;
> +	struct stm32_qspi *qspi = flash->qspi;
> +	struct stm32_qspi_cmd cmd;
> +	int err;
> +
> +	dev_dbg(qspi->dev, "read(%#.2x): buf:%p from:%#.8x len:%#x\n",
> +		nor->read_opcode, buf, (u32)from, len);
> +
> +	memset(&cmd, 0, sizeof(cmd));
> +	cmd.opcode = nor->read_opcode;
> +	cmd.addr_width = nor->addr_width;
> +	cmd.addr = (u32)from;
> +	cmd.tx_data = true;
> +	cmd.dummy = nor->read_dummy;
> +	cmd.len = len;
> +	cmd.buf = buf;
> +	cmd.qspimode = flash->read_mode;
> +
> +	stm32_qspi_set_framemode(nor, &cmd, true);
> +	err = stm32_qspi_send(flash, &cmd);
> +
> +	return err ? err : len;
> +}
> +
> +static ssize_t stm32_qspi_write(struct spi_nor *nor, loff_t to, size_t len,
> +				const u_char *buf)
> +{
> +	struct stm32_qspi_flash *flash = nor->priv;
> +	struct device *dev = flash->qspi->dev;
> +	struct stm32_qspi_cmd cmd;
> +	int err;
> +
> +	dev_dbg(dev, "write(%#.2x): buf:%p to:%#.8x len:%#x\n",
> +		nor->program_opcode, buf, (u32)to, len);
> +
> +	memset(&cmd, 0, sizeof(cmd));
> +	cmd.opcode = nor->program_opcode;
> +	cmd.addr_width = nor->addr_width;
> +	cmd.addr = (u32)to;
> +	cmd.tx_data = true;
> +	cmd.len = len;
> +	cmd.buf = (void *)buf;
> +	cmd.qspimode = CCR_FMODE_INDW;
> +
> +	stm32_qspi_set_framemode(nor, &cmd, false);
> +	err = stm32_qspi_send(flash, &cmd);
> +
> +	return err ? err : len;
> +}
> +
> +static int stm32_qspi_erase(struct spi_nor *nor, loff_t offs)
> +{
> +	struct stm32_qspi_flash *flash = nor->priv;
> +	struct device *dev = flash->qspi->dev;
> +	struct stm32_qspi_cmd cmd;
> +
> +	dev_dbg(dev, "erase(%#.2x):offs:%#x\n", nor->erase_opcode, (u32)offs);
> +
> +	memset(&cmd, 0, sizeof(cmd));
> +	cmd.opcode = nor->erase_opcode;
> +	cmd.addr_width = nor->addr_width;
> +	cmd.addr = (u32)offs;
> +	cmd.qspimode = CCR_FMODE_INDW;
> +
> +	stm32_qspi_set_framemode(nor, &cmd, false);
> +
> +	return stm32_qspi_send(flash, &cmd);
> +}
> +
> +static irqreturn_t stm32_qspi_irq(int irq, void *dev_id)
> +{
> +	struct stm32_qspi *qspi = (struct stm32_qspi *)dev_id;
> +	u32 cr, sr, fcr = 0;
> +
> +	cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
> +	sr = readl_relaxed(qspi->io_base + QUADSPI_SR);
> +
> +	if ((cr & CR_TCIE) && (sr & SR_TCF)) {
> +		/* tx complete */
> +		fcr |= FCR_CTCF;
> +		complete(&qspi->cmd_completion);
> +	} else {
> +		dev_info_ratelimited(qspi->dev, "spurious interrupt\n");
> +	}
> +
> +	writel_relaxed(fcr, qspi->io_base + QUADSPI_FCR);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int stm32_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
> +{
> +	struct stm32_qspi_flash *flash = nor->priv;
> +	struct stm32_qspi *qspi = flash->qspi;
> +
> +	mutex_lock(&qspi->lock);
> +	return 0;
> +}
> +
> +static void stm32_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
> +{
> +	struct stm32_qspi_flash *flash = nor->priv;
> +	struct stm32_qspi *qspi = flash->qspi;
> +
> +	mutex_unlock(&qspi->lock);
> +}
> +
> +static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
> +				  struct device_node *np)
> +{
> +	u32 width, flash_read, presc, cs_num, max_rate = 0;
> +	struct stm32_qspi_flash *flash;
> +	struct mtd_info *mtd;
> +	int ret;
> +
> +	of_property_read_u32(np, "reg", &cs_num);
> +	if (cs_num >= STM32_MAX_NORCHIP)
> +		return -EINVAL;
> +
> +	of_property_read_u32(np, "spi-max-frequency", &max_rate);
> +	if (!max_rate)
> +		return -EINVAL;
> +
> +	presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
> +
> +	if (of_property_read_u32(np, "spi-rx-bus-width", &width))
> +		width = 1;
> +
> +	if (width == 4)
> +		flash_read = SPI_NOR_QUAD;
> +	else if (width == 2)
> +		flash_read = SPI_NOR_DUAL;
> +	else if (width == 1)
> +		flash_read = SPI_NOR_NORMAL;
> +	else
> +		return -EINVAL;
> +
> +	flash = &qspi->flash[cs_num];
> +	flash->qspi = qspi;
> +	flash->cs = cs_num;
> +	flash->presc = presc;
> +
> +	flash->nor.dev = qspi->dev;
> +	spi_nor_set_flash_node(&flash->nor, np);
> +	flash->nor.priv = flash;
> +	mtd = &flash->nor.mtd;
> +	mtd->priv = &flash->nor;

This line is not needed: spi_nor_scan() already initializes mtd->priv
with the nor pointer. Not blocking anyway.

> +
> +	flash->nor.read = stm32_qspi_read;
> +	flash->nor.write = stm32_qspi_write;
> +	flash->nor.erase = stm32_qspi_erase;
> +	flash->nor.read_reg = stm32_qspi_read_reg;
> +	flash->nor.write_reg = stm32_qspi_write_reg;
> +	flash->nor.prepare = stm32_qspi_prep;
> +	flash->nor.unprepare = stm32_qspi_unprep;

A shared comment: your implementations of all those handlers seem good
to me. I didn't notice any particular mistake or issue :)

> +
> +	writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
> +
> +	writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN | CR_SSHIFT
> +		       | CR_EN, qspi->io_base + QUADSPI_CR);
> +
> +	/*
> +	 * in stm32 qspi controller, QUADSPI_DCR register has a fsize field
> +	 * which define the size of nor flash.
> +	 * if fsize is NULL, the controller can't sent spi-nor command.
> +	 * set a temporary value just to discover the nor flash with
> +	 * "spi_nor_scan". After, the right value (mtd->size) can be set.
> +	 */
> +	flash->fsize = FSIZE_VAL(SZ_1K);
> +
> +	ret = spi_nor_scan(&flash->nor, NULL, flash_read);
> +	if (ret) {
> +		dev_err(qspi->dev, "device scan failed\n");

At this point, (flash->qspi != NULL)

Hence in the control flow, once you have returned from
stm32_qspi__flash_setup() back into stm32_qspi_probe(), you handle the
reported error calling stm32_qspi_mtd_free().

This latest function test flash->qspi, which is not NULL, before calling
mtd_device_unregister(). However you did not register flash->nor.mtd at all.

Though not critical, can you fix this anyway, please?
I think, it's more a potential bug than a costmetic issue.

> +		return ret;
> +	}
> +
> +	flash->fsize = FSIZE_VAL(mtd->size);
> +
> +	flash->read_mode = CCR_FMODE_MM;
> +	if (mtd->size > qspi->mm_size)
> +		flash->read_mode = CCR_FMODE_INDR;
> +
> +	writel_relaxed(DCR_CSHT(1), qspi->io_base + QUADSPI_DCR);
> +
> +	ret = mtd_device_register(mtd, NULL, 0);
> +	if (ret) {
> +		dev_err(qspi->dev, "mtd device parse failed\n");

Almost the same mistake here: you shouldn't call mtd_device_unregister()
later from stm32_qspi_mtd_free() if mtd_devire_register() fails here.

> +		return ret;
> +	}
> +
> +	dev_dbg(qspi->dev, "read mm:%s cs:%d bus:%d\n",
> +		flash->read_mode == CCR_FMODE_MM ? "yes" : "no", cs_num, width);
> +
> +	return 0;
> +}
> +
> +static void stm32_qspi_mtd_free(struct stm32_qspi *qspi)
> +{
> +	int i;
> +
> +	for (i = 0; i < STM32_MAX_NORCHIP; i++) {
> +		if (qspi->flash[i].qspi)
> +			mtd_device_unregister(&qspi->flash[i].nor.mtd);
> +	}

IMHO, I guess braces {} in the for() loop are not needed here. So if
checkpatch doesn't complain it's ok for me, you can leave it as is if
you want. Otherwise, please fix!

> +}
> +
> +static int stm32_qspi_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *flash_np;
> +	struct reset_control *rstc;
> +	struct stm32_qspi *qspi;
> +	struct resource *res;
> +	int ret, irq;
> +
> +	qspi = devm_kzalloc(dev, sizeof(*qspi), GFP_KERNEL);
> +	if (!qspi)
> +		return -ENOMEM;
> +
> +	qspi->nor_num = of_get_child_count(dev->of_node);
> +	if (!qspi->nor_num || qspi->nor_num > STM32_MAX_NORCHIP)
> +		return -ENODEV;
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qspi");
> +	qspi->io_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(qspi->io_base))
> +		return PTR_ERR(qspi->io_base);
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qspi_mm");
> +	qspi->mm_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(qspi->mm_base))
> +		return PTR_ERR(qspi->mm_base);
> +
> +	qspi->mm_size = resource_size(res);
> +
> +	irq = platform_get_irq(pdev, 0);
> +	ret = devm_request_irq(dev, irq, stm32_qspi_irq, 0,
> +			       dev_name(dev), qspi);
> +	if (ret) {
> +		dev_err(dev, "failed to request irq\n");
> +		return ret;
> +	}
> +
> +	init_completion(&qspi->cmd_completion);
> +
> +	qspi->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(qspi->clk))
> +		return PTR_ERR(qspi->clk);
> +
> +	qspi->clk_rate = clk_get_rate(qspi->clk);
> +	if (!qspi->clk_rate)
> +		return -EINVAL;
> +
> +	ret = clk_prepare_enable(qspi->clk);
> +	if (ret) {
> +		dev_err(dev, "can not enable the clock\n");
> +		return ret;
> +	}
> +
> +	rstc = devm_reset_control_get(dev, NULL);
> +	if (!IS_ERR(rstc)) {
> +		reset_control_assert(rstc);
> +		udelay(2);
> +		reset_control_deassert(rstc);
> +	}
> +
> +	qspi->dev = dev;
> +	platform_set_drvdata(pdev, qspi);
> +	mutex_init(&qspi->lock);
> +
> +	for_each_available_child_of_node(dev->of_node, flash_np) {
> +		ret = stm32_qspi_flash_setup(qspi, flash_np);
> +		if (ret) {
> +			dev_err(dev, "unable to setup flash chip\n");
> +			goto err_flash;
> +		}
> +	}
> +
> +	return 0;
> +
> +err_flash:
> +	mutex_destroy(&qspi->lock);
> +	stm32_qspi_mtd_free(qspi);
> +
> +	clk_disable_unprepare(qspi->clk);
> +	return ret;
> +}
> +
> +static int stm32_qspi_remove(struct platform_device *pdev)
> +{
> +	struct stm32_qspi *qspi = platform_get_drvdata(pdev);
> +
> +	/* disable qspi */
> +	writel_relaxed(0, qspi->io_base + QUADSPI_CR);
> +
> +	stm32_qspi_mtd_free(qspi);
> +	mutex_destroy(&qspi->lock);
> +
> +	clk_disable_unprepare(qspi->clk);
> +	return 0;
> +}
> +
> +static const struct of_device_id stm32_qspi_match[] = {
> +	{.compatible = "st,stm32f469-qspi"},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, stm32_qspi_match);
> +
> +static struct platform_driver stm32_qspi_driver = {
> +	.probe	= stm32_qspi_probe,
> +	.remove	= stm32_qspi_remove,
> +	.driver	= {
> +		.name = "stm32-quadspi",
> +		.of_match_table = stm32_qspi_match,
> +	},
> +};
> +module_platform_driver(stm32_qspi_driver);
> +
> +MODULE_ALIAS("platform:" DRIVER_NAME);
> +MODULE_AUTHOR("Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>");
> +MODULE_DESCRIPTION("STMicroelectronics STM32 quad spi driver");
> +MODULE_LICENSE("GPL v2");
> 

Best regards,

Cyrille
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 3/4] bluetooth: hci_uart: add LL protocol serdev driver support
From: Marcel Holtmann @ 2017-04-12 20:20 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-bluetooth, linux-arm-kernel, Gustavo F. Padovan,
	Johan Hedberg, Mark Rutland, Wei Xu, Eyal Reizer, Satish Patel,
	netdev, devicetree
In-Reply-To: <20170407143516.9945-1-robh@kernel.org>

Hi Rob,

> Turns out that the LL protocol and the TI-ST are the same thing AFAICT.
> The TI-ST adds firmware loading, GPIO control, and shared access for
> NFC, FM radio, etc. For now, we're only implementing what is needed for
> BT. This mirrors other drivers like BCM and Intel, but uses the new
> serdev bus.
> 
> The firmware loading is greatly simplified by using existing
> infrastructure to send commands. It may be a bit slower than the
> original code using synchronous functions, but the real bottleneck is
> likely doing firmware load at 115.2kbps.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> Cc: Marcel Holtmann <marcel@holtmann.org>
> Cc: Gustavo Padovan <gustavo@padovan.org>
> Cc: Johan Hedberg <johan.hedberg@gmail.com>
> Cc: linux-bluetooth@vger.kernel.org
> ---
> v2:
> - Use IS_ENABLED() to fix module build
> 
> drivers/bluetooth/hci_ll.c | 261 ++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 260 insertions(+), 1 deletion(-)

can you re-send any missing patch on top of today's bluetooth-next tree.

Regards

Marcel

^ permalink raw reply

* Re: [PATCHv3 00/10] Nokia H4+ support
From: Marcel Holtmann @ 2017-04-12 20:19 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Sebastian Reichel, Gustavo F. Padovan, Johan Hedberg,
	Samuel Thibault, Pavel Machek, Tony Lindgren, Jiri Slaby,
	Mark Rutland, open list:BLUETOOTH DRIVERS,
	linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	David S. Miller, Rob Herring
In-Reply-To: <20170411140637.GA4388-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>

Hi Sebastian,

>>>>>>>> Here is PATCHv3 for the Nokia bluetooth patchset. I addressed all comments from
>>>>>>>> Rob and Pavel regarding the serdev patches and dropped the *.dts patches, since
>>>>>>>> they were queued by Tony. I also changed the patch order, so that the serdev
>>>>>>>> patches come first. All of them have Acked-by from Rob, so I think it makes
>>>>>>>> sense to merge them to serdev subsystem (now) and provide an immutable branch
>>>>>>>> for the bluetooth subsystem.
>>>>>>> 
>>>>>>> Greg doesn't read cover letters generally and since the serdev patches
>>>>>>> are Cc rather than To him, he's probably not planning to pick them up.
>>>>>> 
>>>>>> I wonder actually if we should merge all of these via bluetooth-next
>>>>>> tree with proper Ack from Greg. However it would be good to also get
>>>>>> buy in from Dave for merging this ultimately through net-next.
>>>>> 
>>>>> I don't really care where it goes.  I can take the whole thing in my
>>>>> tty/serial tree now if no one objects and I get an ack from the relevant
>>>>> maintainers {hint...}
>>>> 
>>>> I think it is better if it goes thru BT tree. I have another driver
>>>> converted that is dependent on this series. There's a couple other
>>>> serdev changes on the list too, but this shouldn't depend on them.
>>> 
>>> Is this waiting for something, or could it be queued to
>>> bluetooth-next then? It would be nice to finally have
>>> this in 4.12 :)
>> 
>> I would prefer if we can get an ACK from Greg. Then I merge it through the bluetooth-next tree.
> 
> Sorry thought this was coming through mine:
> 	Acked-by: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> 
> Merge away!

so I have applied patches 1-8 to bluetooth-next tree.

The last 2 I left out since they do cause build issues on non-DT platforms. We need to be able to build the driver on all platforms so that sanity compile checks happen all the time.

  CC      drivers/bluetooth/hci_nokia.o
drivers/bluetooth/hci_nokia.c:802:34: error: array type has incomplete element type ‘struct of_device_id’
 static const struct of_device_id nokia_bluetooth_of_match[] = {
                                  ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/bluetooth/hci_nokia.c:803:4: error: field name not in record or union initializer
  { .compatible = "nokia,h4p-bluetooth", },
    ^
drivers/bluetooth/hci_nokia.c:803:4: note: (near initialization for ‘nokia_bluetooth_of_match’)
drivers/bluetooth/hci_nokia.c:815:21: error: implicit declaration of function ‘of_match_ptr’ [-Werror=implicit-function-declaration]
   .of_match_table = of_match_ptr(nokia_bluetooth_of_match),
                     ^~~~~~~~~~~~
drivers/bluetooth/hci_nokia.c:802:34: warning: ‘nokia_bluetooth_of_match’ defined but not used [-Wunused-variable]
 static const struct of_device_id nokia_bluetooth_of_match[] = {
                                  ^~~~~~~~~~~~~~~~~~~~~~~~

Regards

Marcel

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

^ permalink raw reply

* Re: [PATCH v5 04/10] drm/stm: Add STM32 LTDC driver
From: Benjamin Gaignard @ 2017-04-12 18:58 UTC (permalink / raw)
  To: Eric Anholt
  Cc: Mark Rutland, devicetree, Alexandre TORGUE, Arnd Bergmann,
	linux-arm-kernel, Russell King, Rob Herring, Philippe Cornu,
	Yannick Fertre, kernel, dri-devel@lists.freedesktop.org,
	Maxime Coquelin, Mickael Reulier, Vincent Abriou, Fabien Dessenne,
	Gabriel FERNANDEZ, Benjamin Gaignard
In-Reply-To: <87wpaqeks2.fsf@eliezer.anholt.net>

2017-04-11 22:45 GMT+02:00 Eric Anholt <eric@anholt.net>:
> Yannick Fertre <yannick.fertre@st.com> writes:
>
>> This controller provides output signals to interface directly a variety
>> of LCD and TFT panels. These output signals are: RGB signals
>> (up to 24bpp), vertical & horizontal synchronisations, data enable and
>> the pixel clock.
>>
>> Signed-off-by: Yannick Fertre <yannick.fertre@st.com>
>> ---
>>  drivers/gpu/drm/Kconfig      |    3 +-
>>  drivers/gpu/drm/Makefile     |    1 +
>>  drivers/gpu/drm/stm/Kconfig  |   16 +
>>  drivers/gpu/drm/stm/Makefile |    7 +
>>  drivers/gpu/drm/stm/drv.c    |  221 ++++++++
>>  drivers/gpu/drm/stm/ltdc.c   | 1210 ++++++++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/stm/ltdc.h   |   40 ++
>>  7 files changed, 1497 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/gpu/drm/stm/Kconfig
>>  create mode 100644 drivers/gpu/drm/stm/Makefile
>>  create mode 100644 drivers/gpu/drm/stm/drv.c
>>  create mode 100644 drivers/gpu/drm/stm/ltdc.c
>>  create mode 100644 drivers/gpu/drm/stm/ltdc.h
>>
>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index 78d7fc0..dd5762a 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -203,7 +203,6 @@ config DRM_VGEM
>>         as used by Mesa's software renderer for enhanced performance.
>>         If M is selected the module will be called vgem.
>>
>> -
>
> Stray whitespace change.
>
> With this removed, the driver is:
>
> Reviewed-by: Eric Anholt <eric@anholt.net>
>
> Apologies for the delay in the second review offered.  The remainder of
> my comments are little cleanups, all of which I think are optional and
> fine to do after the code lands.
>
> You should probably update MAINTAINERS for your new driver.  If you'd
> like to maintain this driver in the drm-misc small drivers collection
> (https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html),
> send a follow-up patch to the list to add the MAINTAINERS entry, and I
> can get that and patches 1-4 merged.  Once you have a few more patches
> in, we can add you to the drm-misc committers crew so you can merge
> directly after getting review.

Since we are moving sti driver to drm-misc it makes sense to move all
STMicroelectronic SoC display drivers to it.

>
> I'll also take this moment to plug something: Please feel welcome to
> review other people's driver patches on the list.  You've built
> something nice here, and probably learned a lot of lessons along the way
> that you could share with others.  (I just found out about
> of_reset_control in reviewing your code, and I wish I had known about it
> back when I was landing vc4!)
>
>> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
>> new file mode 100644
>> index 0000000..922f021
>> --- /dev/null
>> +++ b/drivers/gpu/drm/stm/ltdc.c
>
>> +static void ltdc_crtc_disable(struct drm_crtc *crtc)
>> +{
>> +     struct ltdc_device *ldev = crtc_to_ltdc(crtc);
>> +     struct drm_pending_vblank_event *event = crtc->state->event;
>> +
>> +     DRM_DEBUG_DRIVER("\n");
>> +
>> +     if (!crtc->enabled) {
>> +             DRM_DEBUG_DRIVER("already disabled\n");
>> +             return;
>> +     }
>
> I think this crtc->enabled is a given for the disable() being called.
>
>> +
>> +     drm_crtc_vblank_off(crtc);
>> +
>> +     /* disable LTDC */
>> +     reg_clear(ldev->regs, LTDC_GCR, GCR_LTDCEN);
>> +
>> +     /* disable IRQ */
>> +     reg_clear(ldev->regs, LTDC_IER, IER_RRIE | IER_FUIE | IER_TERRIE);
>> +
>> +     /* immediately commit disable of layers before switching off LTDC */
>> +     reg_set(ldev->regs, LTDC_SRCR, SRCR_IMR);
>> +
>> +     if (event) {
>> +             crtc->state->event = NULL;
>> +
>> +             spin_lock_irq(&crtc->dev->event_lock);
>> +             if (crtc->state->active && drm_crtc_vblank_get(crtc) == 0)
>> +                     drm_crtc_arm_vblank_event(crtc, event);
>> +             else
>> +                     drm_crtc_send_vblank_event(crtc, event);
>> +             spin_unlock_irq(&crtc->dev->event_lock);
>> +     }
>
> I believe that we're guaranteed that crtc->state->event is NULL in the
> disable call, since your atomic_flush() already armed or sent the event
> and NULLed out the pointer.
>
>> +struct drm_connector *ltdc_rgb_connector_create(struct drm_device *ddev)
>> +{
>> +     struct drm_connector *connector;
>> +     int err;
>> +
>> +     connector = devm_kzalloc(ddev->dev, sizeof(*connector), GFP_KERNEL);
>> +     if (!connector) {
>> +             DRM_ERROR("Failed to allocate connector\n");
>> +             return NULL;
>> +     }
>> +
>> +     connector->polled = DRM_CONNECTOR_POLL_HPD;
>> +
>> +     err = drm_connector_init(ddev, connector, &ltdc_rgb_connector_funcs,
>> +                              DRM_MODE_CONNECTOR_LVDS);
>
> I think DRM_MODE_CONNECTOR_DPI (and _ENCODER_DPI) are slightly more
> accurate descriptions, if I'm interpreting your pinmux setup right.
> It's cosmetic, though.
>
>> +static struct drm_panel *ltdc_get_panel(struct drm_device *ddev)
>> +{
>> +     struct device *dev = ddev->dev;
>> +     struct device_node *np = dev->of_node;
>> +     struct device_node *entity, *port = NULL;
>> +     struct drm_panel *panel = NULL;
>> +
>> +     DRM_DEBUG_DRIVER("\n");
>> +
>> +     /*
>> +      * Parse ltdc node to get remote port and find RGB panel / HDMI slave
>> +      * If a dsi or a bridge (hdmi, lvds...) is connected to ltdc,
>> +      * a remote port & RGB panel will not be found.
>> +      */
>> +     for_each_endpoint_of_node(np, entity) {
>> +             if (!of_device_is_available(entity))
>> +                     continue;
>> +
>> +             port = of_graph_get_remote_port_parent(entity);
>> +             if (port) {
>> +                     panel = of_drm_find_panel(port);
>> +                     of_node_put(port);
>> +                     if (panel) {
>> +                             DRM_DEBUG_DRIVER("remote panel %s\n",
>> +                                              port->full_name);
>> +                     } else {
>> +                             DRM_DEBUG_DRIVER("panel missing\n");
>> +                             of_node_put(entity);
>> +                     }
>> +             }
>> +     }
>
> Future work: You may find the new drm_of_find_panel_or_bridge() useful
> to drop this loop.
>
>> +
>> +     return panel;
>> +}
>> +
>> +int ltdc_load(struct drm_device *ddev)
>> +{
>> +     struct platform_device *pdev = to_platform_device(ddev->dev);
>> +     struct ltdc_device *ldev = ddev->dev_private;
>> +     struct device *dev = ddev->dev;
>> +     struct device_node *np = dev->of_node;
>> +     struct drm_encoder *encoder;
>> +     struct drm_connector *connector = NULL;
>> +     struct drm_crtc *crtc;
>> +     struct reset_control *rstc;
>> +     struct resource res;
>> +     int irq, ret, i;
>> +
>> +     DRM_DEBUG_DRIVER("\n");
>> +
>> +     ldev->panel = ltdc_get_panel(ddev);
>> +     if (!ldev->panel)
>> +             return -EPROBE_DEFER;
>> +
>> +     rstc = of_reset_control_get(np, NULL);
>> +
>> +     mutex_init(&ldev->err_lock);
>> +
>> +     ldev->pixel_clk = devm_clk_get(dev, "lcd");
>> +     if (IS_ERR(ldev->pixel_clk)) {
>> +             DRM_ERROR("Unable to get lcd clock\n");
>> +             return -ENODEV;
>> +     }
>> +
>> +     if (clk_prepare_enable(ldev->pixel_clk)) {
>> +             DRM_ERROR("Unable to prepare pixel clock\n");
>> +             return -ENODEV;
>> +     }
>
> Future work: You may want to move the pixel clock enable into the CRTC's
> .enable() and disable in .disable().  It sounded in previous versions
> like the HW uses that clock for all register accesses, so you'd need to
> protect a couple of other places, but that should save power when the
> device is off, right?
>
>> +
>> +     if (of_address_to_resource(np, 0, &res)) {
>> +             DRM_ERROR("Unable to get resource\n");
>> +             return -ENODEV;
>> +     }
>> +
>> +     ldev->regs = devm_ioremap_resource(dev, &res);
>> +     if (IS_ERR(ldev->regs)) {
>> +             DRM_ERROR("Unable to get ltdc registers\n");
>> +             return PTR_ERR(ldev->regs);
>> +     }
>> +
>> +     for (i = 0; i < MAX_IRQ; i++) {
>> +             irq = platform_get_irq(pdev, i);
>> +             if (irq < 0)
>> +                     continue;
>> +
>> +             ret = devm_request_threaded_irq(dev, irq, ltdc_irq,
>> +                                             ltdc_irq_thread, IRQF_ONESHOT,
>> +                                             dev_name(dev), ddev);
>> +             if (ret) {
>> +                     DRM_ERROR("Failed to register LTDC interrupt\n");
>> +                     return ret;
>> +             }
>> +     }
>> +
>> +     if (!IS_ERR(rstc))
>> +             reset_control_deassert(rstc);
>> +
>> +     /* Disable interrupts */
>> +     reg_clear(ldev->regs, LTDC_IER,
>> +               IER_LIE | IER_RRIE | IER_FUIE | IER_TERRIE);
>> +
>> +     ret = ltdc_get_caps(ddev);
>> +     if (ret) {
>> +             DRM_ERROR("hardware identifier (0x%08x) not supported!\n",
>> +                       ldev->caps.hw_version);
>> +             return ret;
>> +     }
>> +
>> +     DRM_INFO("ltdc hw version 0x%08x - ready\n", ldev->caps.hw_version);
>> +
>> +     if (ltdc_create_encoders(ddev)) {
>> +             DRM_ERROR("Failed to create encoders\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     if (ldev->panel) {
>> +             encoder = ltdc_rgb_encoder_find(ddev);
>> +             if (!encoder) {
>> +                     DRM_ERROR("Failed to find RGB encoder\n");
>> +                     ret = -EINVAL;
>> +                     goto err;
>> +             }
>
> Given that ltdc_create_encoders() only does work if ldev->panel, its
> body could probably be moved in here and then ltdc_rgb_encoder_find
> could be dropped.
>
>> +
>> +             connector = ltdc_rgb_connector_create(ddev);
>> +             if (!connector) {
>> +                     DRM_ERROR("Failed to create RGB connector\n");
>> +                     ret = -EINVAL;
>> +                     goto err;
>> +             }
>> +
>> +             ret = drm_mode_connector_attach_encoder(connector, encoder);
>> +             if (ret) {
>> +                     DRM_ERROR("Failed to attach connector to encoder\n");
>> +                     goto err;
>> +             }
>> +
>> +             drm_panel_attach(ldev->panel, connector);
>> +     }
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH v2 3/3] of: Make of_match_node() an inline stub for CONFIG_OF=n
From: kbuild test robot @ 2017-04-12 18:58 UTC (permalink / raw)
  Cc: kbuild-all-JC7UmRfGjtg, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	andrew-g2DYL2Zd6BY,
	vivien.didelot-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/, Florian Fainelli,
	Lee Jones, Nicolas Ferre, Rob Herring, Frank Rowand,
	open list:NETWORKING DRIVERS,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE
In-Reply-To: <20170412044156.17351-4-f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 4084 bytes --]

Hi Florian,

[auto build test ERROR on ljones-mfd/for-mfd-next]
[also build test ERROR on v4.11-rc6 next-20170412]
[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/Florian-Fainelli/of-Make-of_match_node-an-inline-stub-for-CONFIG_OF-n/20170412-160916
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: arm-s3c2410_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/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=arm 

All errors (new ones prefixed by >>):

   drivers/i2c/busses/i2c-s3c2410.c: In function 's3c24xx_get_device_quirks':
>> drivers/i2c/busses/i2c-s3c2410.c:174:25: error: 's3c24xx_i2c_match' undeclared (first use in this function)
      match = of_match_node(s3c24xx_i2c_match, pdev->dev.of_node);
                            ^~~~~~~~~~~~~~~~~
   drivers/i2c/busses/i2c-s3c2410.c:174:25: note: each undeclared identifier is reported only once for each function it appears in
--
   drivers/watchdog/s3c2410_wdt.c: In function 's3c2410_get_wdt_drv_data':
>> drivers/watchdog/s3c2410_wdt.c:515:25: error: 's3c2410_wdt_match' undeclared (first use in this function)
      match = of_match_node(s3c2410_wdt_match, pdev->dev.of_node);
                            ^~~~~~~~~~~~~~~~~
   drivers/watchdog/s3c2410_wdt.c:515:25: note: each undeclared identifier is reported only once for each function it appears in

vim +/s3c24xx_i2c_match +174 drivers/i2c/busses/i2c-s3c2410.c

faf93ff6 Giridhar Maruthy    2013-01-24  158  	  .data = (void *)(QUIRK_S3C2440 | QUIRK_NO_GPIO) },
117053f7 Vasanth Ananthan    2013-11-11  159  	{ .compatible = "samsung,exynos5-sata-phy-i2c",
117053f7 Vasanth Ananthan    2013-11-11  160  	  .data = (void *)(QUIRK_S3C2440 | QUIRK_POLL | QUIRK_NO_GPIO) },
27452498 Karol Lewandowski   2012-04-23  161  	{},
27452498 Karol Lewandowski   2012-04-23  162  };
27452498 Karol Lewandowski   2012-04-23  163  MODULE_DEVICE_TABLE(of, s3c24xx_i2c_match);
27452498 Karol Lewandowski   2012-04-23  164  #endif
^1da177e Linus Torvalds      2005-04-16  165  
ec7c34a4 Krzysztof Kozlowski 2016-04-21  166  /*
27452498 Karol Lewandowski   2012-04-23  167   * Get controller type either from device tree or platform device variant.
^1da177e Linus Torvalds      2005-04-16  168   */
5f1b1155 Pankaj Dubey        2014-01-15  169  static inline kernel_ulong_t s3c24xx_get_device_quirks(struct platform_device *pdev)
^1da177e Linus Torvalds      2005-04-16  170  {
27452498 Karol Lewandowski   2012-04-23  171  	if (pdev->dev.of_node) {
27452498 Karol Lewandowski   2012-04-23  172  		const struct of_device_id *match;
0915833b Krzysztof Kozlowski 2016-04-21  173  
b900ba4c Karol Lewandowski   2012-05-30 @174  		match = of_match_node(s3c24xx_i2c_match, pdev->dev.of_node);
5f1b1155 Pankaj Dubey        2014-01-15  175  		return (kernel_ulong_t)match->data;
27452498 Karol Lewandowski   2012-04-23  176  	}
5a5f5080 Thomas Abraham      2011-09-13  177  
27452498 Karol Lewandowski   2012-04-23  178  	return platform_get_device_id(pdev)->driver_data;
^1da177e Linus Torvalds      2005-04-16  179  }
^1da177e Linus Torvalds      2005-04-16  180  
ec7c34a4 Krzysztof Kozlowski 2016-04-21  181  /*
ec7c34a4 Krzysztof Kozlowski 2016-04-21  182   * Complete the message and wake up the caller, using the given return code,

:::::: The code at line 174 was first introduced by commit
:::::: b900ba4c1513a8c9a2fab8dca4cc6f50b17d6861 i2c: s3c2410: Fix pointer type passed to of_match_node()

:::::: TO: Karol Lewandowski <k.lewandowsk-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
:::::: CC: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 22646 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox