* [PATCH 4/5] ARM64: dts: meson-gxm: add GXM specific USB configuration
From: Martin Blumenstingl @ 2016-11-26 14:56 UTC (permalink / raw)
To: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, kishon-l0cyMroinI0,
khilman-rdvid1DuHRBWk0Htik3J/w, carlo-KA+7E9HrN00dnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
Cc: catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
narmstrong-rdvid1DuHRBWk0Htik3J/w, Martin Blumenstingl
In-Reply-To: <20161126145635.24488-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
The USB configuration on GXM is slightly different than on GXL. On GXM
the dwc2 controller is limited (via GHWCFG2_OP_MODE_MASK) to "device
mode".
The dwc3 controller's internal hub has 3 ports (instead of 2 on GXL)
enabled. It's hardware configuration limits it (via DWC3_GHWPARAMS0)
to host mode only.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
arch/arm64/boot/dts/amlogic/meson-gxm.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
index 2b1d276e..0a6b224 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
@@ -121,3 +121,13 @@
clock-indices = <0 1>;
clock-output-names = "vbig", "vlittle";
};
+
+&usb3_phy0 {
+ /* dwc3 on GXM enables 3 USB ports on the internal hub */
+ phys = <&usb2_phys 0>, <&usb2_phys 1>, <&usb2_phys 2>;
+};
+
+&usb1 {
+ /* the dwc2 hardware configuration on GXM only allows device mode */
+ dr_mode = "device";
+};
--
2.10.2
--
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 5/5] ARM64: dts: meson-gx-p23x-q20x: enable USB on P23x and Q20x boards
From: Martin Blumenstingl @ 2016-11-26 14:56 UTC (permalink / raw)
To: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, kishon-l0cyMroinI0,
khilman-rdvid1DuHRBWk0Htik3J/w, carlo-KA+7E9HrN00dnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
Cc: catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
narmstrong-rdvid1DuHRBWk0Htik3J/w, Martin Blumenstingl
In-Reply-To: <20161126145635.24488-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
All four devices are only using the dwc3 controller. The actual ports
are provided by dwc3's internal USB hub.
The implementation on P230, P231 and Q201 seems identical: the USB VBUS
supply seems to be hard-wired, while on Q200 the USB VBUS is provided
by GPIOAO_5.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi | 12 ++++++++++++
arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts | 17 +++++++++++++++++
2 files changed, 29 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi
index 7a078be..360c91d 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi
@@ -188,3 +188,15 @@
ðmac {
status = "okay";
};
+
+&usb2_phys {
+ status = "okay";
+};
+
+&usb3_phy0 {
+ status = "okay";
+};
+
+&usb0 {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts
index 5dbc660..6c28e87 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts
@@ -49,6 +49,19 @@
/ {
compatible = "amlogic,q200", "amlogic,s912", "amlogic,meson-gxm";
model = "Amlogic Meson GXM (S912) Q200 Development Board";
+
+ usb_pwr: regulator-usb-pwrs {
+ compatible = "regulator-fixed";
+
+ regulator-name = "USB_PWR";
+
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+
+ /* signal name in schematic: USB_PWR_EN */
+ gpio = <&gpio GPIODV_24 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
};
/* Q200 has exclusive choice between internal or external PHY */
@@ -75,3 +88,7 @@
max-speed = <1000>;
};
};
+
+&usb3_phy0 {
+ phy-supply = <&usb_pwr>;
+};
--
2.10.2
--
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 00/39] mtd: nand: denali: 2nd round of Denali NAND IP patch bomb
From: Masahiro Yamada @ 2016-11-26 18:05 UTC (permalink / raw)
To: linux-mtd
Cc: Masahiro Yamada, devicetree, linux-kernel, Boris Brezillon,
Marek Vasut, Brian Norris, Richard Weinberger, David Woodhouse,
Cyrille Pitchen, Rob Herring, Mark Rutland
As I said in the 1st round series, I am tackling on this driver
to use it for my SoCs.
The previous series was just cosmetic things, but this series
includes *real* changes.
After some more cleanups, I will start to add changes that
are really necessary.
One of the biggest problems I want to solve is a bunch of
hard-coded parameters that prevent me from using this driver for
my SoCs.
I will introduce capability flags that are associated with DT
compatible and make platform-dependent parameters overridable.
I still have lots of reworks to get done (so probably 3rd round
series will come), but I hope it is getting better and
I am showing a big picture now.
Masahiro Yamada (39):
mtd: nand: allow to set only one of ECC size and ECC strength from DT
mtd: nand: denali: remove unused CONFIG option and macros
mtd: nand: denali: remove redundant define of BANK(x)
mtd: nand: denali: remove more unused struct members
mtd: nand: denali: fix comment of denali_nand_info::flash_mem
mtd: nand: denali: fix write_oob_data() function
mtd: nand: denali: transfer OOB only when oob_required is set
mtd: nand: denali: introduce capability flag
mtd: nand: denali: fix erased page check code
mtd: nand: denali: remove redundant if conditional of erased_check
mtd: nand: denali: increment ecc_stats.failed by one per error
mtd: nand: denali: return 0 for uncorrectable ECC error
mtd: nand: denali: increment ecc_stats->corrected
mtd: nand: denali: replace uint{8/16/32}_t with u{8/16/32}
mtd: nand: denali: improve readability of handle_ecc()
mtd: nand: denali: rename handle_ecc() to denali_sw_ecc_fixup()
mtd: nand: denali: support HW_ECC_FIXUP capability
mtd: nand: denali: move denali_read_page_raw() above
denali_read_page()
mtd: nand: denali: perform erased check against raw transferred page
mtd: nand: denali_dt: enable HW_ECC_FIXUP capability for DT platform
mtd: nand: denali: support 64bit capable DMA engine
mtd: nand: denali_dt: remove dma-mask DT property
mtd: nand: denali_dt: use pdev instead of ofdev for platform_device
mtd: nand: denali: add NEW_N_BANKS_FORMAT capability
mtd: nand: denali: use nand_chip to hold frequently accessed data
mtd: nand: denali: call nand_set_flash_node() to set DT node
mtd: nand: denali: do not set mtd->name
mtd: nand: denali: move multi NAND fixup code to a helper function
mtd: nand: denali: refactor multi NAND fixup code in more generic way
mtd: nand: denali: set DEVICES_CONNECTED 1 if not set
mtd: nand: denali: remove meaningless writes to read-only registers
mtd: nand: denali: remove unnecessary writes to ECC_CORRECTION
mtd: nand: denali: support 1024 byte ECC step size
mtd: nand: denali: fix the condition for 15 bit ECC strength
mtd: nand: denali: calculate ecc.strength and ecc.bytes generically
mtd: nand: denali: allow to use SoC-specific ECC strength
mtd: nand: denali: support "nand-ecc-strength" DT property
mtd: nand: denali: remove Toshiba, Hynix specific fixup code
mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants
.../devicetree/bindings/mtd/denali-nand.txt | 19 +-
drivers/mtd/nand/Kconfig | 11 -
drivers/mtd/nand/denali.c | 740 ++++++++++++---------
drivers/mtd/nand/denali.h | 84 +--
drivers/mtd/nand/denali_dt.c | 95 ++-
drivers/mtd/nand/denali_pci.c | 2 +
drivers/mtd/nand/nand_base.c | 6 -
7 files changed, 515 insertions(+), 442 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH 22/39] mtd: nand: denali_dt: remove dma-mask DT property
From: Masahiro Yamada @ 2016-11-26 18:06 UTC (permalink / raw)
To: linux-mtd
Cc: Masahiro Yamada, devicetree, linux-kernel, Boris Brezillon,
Marek Vasut, Brian Norris, Richard Weinberger, David Woodhouse,
Cyrille Pitchen, Rob Herring, Mark Rutland
In-Reply-To: <1480183585-592-1-git-send-email-yamada.masahiro@socionext.com>
The driver sets appropriate DMA mask. Delete the "dma-mask" DT
property. Refer to the Link tag for negative opinions for this
binding.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Link: https://lkml.org/lkml/2016/2/8/57
---
Documentation/devicetree/bindings/mtd/denali-nand.txt | 2 --
drivers/mtd/nand/denali_dt.c | 9 ---------
2 files changed, 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/denali-nand.txt b/Documentation/devicetree/bindings/mtd/denali-nand.txt
index b04d03a..603110b 100644
--- a/Documentation/devicetree/bindings/mtd/denali-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/denali-nand.txt
@@ -5,7 +5,6 @@ Required properties:
- reg : should contain registers location and length for data and reg.
- reg-names: Should contain the reg names "nand_data" and "denali_reg"
- interrupts : The interrupt number.
- - dm-mask : DMA bit mask
The device tree may optionally contain sub-nodes describing partitions of the
address space. See partition.txt for more detail.
@@ -19,5 +18,4 @@ nand: nand@ff900000 {
reg = <0xff900000 0x100000>, <0xffb80000 0x10000>;
reg-names = "nand_data", "denali_reg";
interrupts = <0 144 4>;
- dma-mask = <0xffffffff>;
};
diff --git a/drivers/mtd/nand/denali_dt.c b/drivers/mtd/nand/denali_dt.c
index 9dcd203..6a486a7 100644
--- a/drivers/mtd/nand/denali_dt.c
+++ b/drivers/mtd/nand/denali_dt.c
@@ -40,8 +40,6 @@ static const struct of_device_id denali_nand_dt_ids[] = {
MODULE_DEVICE_TABLE(of, denali_nand_dt_ids);
-static u64 denali_dma_mask;
-
static int denali_dt_probe(struct platform_device *ofdev)
{
struct resource *denali_reg, *nand_data;
@@ -79,13 +77,6 @@ static int denali_dt_probe(struct platform_device *ofdev)
if (IS_ERR(denali->flash_mem))
return PTR_ERR(denali->flash_mem);
- if (!of_property_read_u32(ofdev->dev.of_node,
- "dma-mask", (u32 *)&denali_dma_mask)) {
- denali->dev->dma_mask = &denali_dma_mask;
- } else {
- denali->dev->dma_mask = NULL;
- }
-
dt->clk = devm_clk_get(&ofdev->dev, NULL);
if (IS_ERR(dt->clk)) {
dev_err(&ofdev->dev, "no clk available\n");
--
2.7.4
^ permalink raw reply related
* [PATCH 33/39] mtd: nand: denali: support 1024 byte ECC step size
From: Masahiro Yamada @ 2016-11-26 18:06 UTC (permalink / raw)
To: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Masahiro Yamada, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Boris Brezillon, Marek Vasut,
Brian Norris, Richard Weinberger, David Woodhouse,
Cyrille Pitchen, Rob Herring, Mark Rutland
In-Reply-To: <1480183585-592-1-git-send-email-yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
This driver was originally written for the Intel MRST platform with
several platform specific parameters hard-coded. Another thing we
need to fix is the hard-coded ECC step size. Currently, it is
defined as follows:
#define ECC_SECTOR_SIZE 512
(somehow, it is defined in both denali.c and denali.h)
This must be avoided because the Denali IP supports 1024 byte ECC
size as well. Add a new flag DENALI_CAPS_ECC_SIZE_1024. If it is
specified, ecc.size is set to 1024, otherwise set to 512.
We can use "nand-ecc-step-size" DT property to override the ecc.size
if we want, but this capability flag can provide the reasonable
default because it is associated with the DT compatible strings.
Signed-off-by: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
---
.../devicetree/bindings/mtd/denali-nand.txt | 4 ++++
drivers/mtd/nand/denali.c | 26 +++++++++++-----------
drivers/mtd/nand/denali.h | 3 +--
3 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/denali-nand.txt b/Documentation/devicetree/bindings/mtd/denali-nand.txt
index 603110b..e9d5818 100644
--- a/Documentation/devicetree/bindings/mtd/denali-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/denali-nand.txt
@@ -6,6 +6,10 @@ Required properties:
- reg-names: Should contain the reg names "nand_data" and "denali_reg"
- interrupts : The interrupt number.
+Optional properties:
+ - nand-ecc-step-size: must be 512 or 1024. If not specified, default to 512.
+ see nand.txt for details.
+
The device tree may optionally contain sub-nodes describing partitions of the
address space. See partition.txt for more detail.
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 63f7500..5d80f16 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -894,8 +894,6 @@ static bool denali_hw_ecc_fixup(struct denali_nand_info *denali,
return false;
}
-#define ECC_SECTOR_SIZE 512
-
#define ECC_SECTOR(x) (((x) & ECC_ERROR_ADDRESS__SECTOR_NR) >> 12)
#define ECC_BYTE(x) (((x) & ECC_ERROR_ADDRESS__OFFSET))
#define ECC_CORRECTION_VALUE(x) ((x) & ERR_CORRECTION_INFO__BYTEMASK)
@@ -908,6 +906,7 @@ static bool denali_sw_ecc_fixup(struct denali_nand_info *denali, u8 *buf,
{
bool check_erased_page = false;
unsigned int bitflips = 0;
+ unsigned int ecc_size = denali->nand.ecc.size;
u32 err_address, err_correction_info, err_byte, err_sector, err_device,
err_correction_value;
@@ -930,18 +929,18 @@ static bool denali_sw_ecc_fixup(struct denali_nand_info *denali, u8 *buf,
if (ECC_ERROR_CORRECTABLE(err_correction_info)) {
/*
- * If err_byte is larger than ECC_SECTOR_SIZE, means error
+ * If err_byte is larger than ecc_size, means error
* happened in OOB, so we ignore it. It's no need for
* us to correct it err_device is represented the NAND
* error bits are happened in if there are more than
* one NAND connected.
*/
- if (err_byte < ECC_SECTOR_SIZE) {
+ if (err_byte < ecc_size) {
struct mtd_info *mtd =
nand_to_mtd(&denali->nand);
int offset;
- offset = (err_sector * ECC_SECTOR_SIZE + err_byte) *
+ offset = (err_sector * ecc_size + err_byte) *
denali->devnum + err_device;
/* correct the ECC error */
buf[offset] ^= err_correction_value;
@@ -1590,22 +1589,25 @@ int denali_init(struct denali_nand_info *denali)
/* no subpage writes on denali */
chip->options |= NAND_NO_SUBPAGE_WRITE;
+ /* If "nand-ecc-step-size" DT property is specified, respect it */
+ if (!chip->ecc.size)
+ chip->ecc.size = denali->caps & DENALI_CAPS_ECC_SIZE_1024 ?
+ 1024 : 512;
+
/*
* Denali Controller only support 15bit and 8bit ECC in MRST,
* so just let controller do 15bit ECC for MLC and 8bit ECC for
* SLC if possible.
* */
if (!nand_is_slc(chip) &&
- (mtd->oobsize > (denali->bbtskipbytes +
- ECC_15BITS * (mtd->writesize /
- ECC_SECTOR_SIZE)))) {
+ mtd->oobsize > denali->bbtskipbytes +
+ ECC_15BITS * (mtd->writesize / chip->ecc.size)) {
/* if MLC OOB size is large enough, use 15bit ECC*/
chip->ecc.strength = 15;
chip->ecc.bytes = ECC_15BITS;
iowrite32(15, denali->flash_reg + ECC_CORRECTION);
- } else if (mtd->oobsize < (denali->bbtskipbytes +
- ECC_8BITS * (mtd->writesize /
- ECC_SECTOR_SIZE))) {
+ } else if (mtd->oobsize <
+ denali->bbtskipbytes + ECC_8BITS * (mtd->writesize / chip->ecc.size)) {
pr_err("Your NAND chip OOB is not large enough to contain 8bit ECC correction codes");
goto failed_req_irq;
} else {
@@ -1616,8 +1618,6 @@ int denali_init(struct denali_nand_info *denali)
mtd_set_ooblayout(mtd, &denali_ooblayout_ops);
- /* override the default read operations */
- chip->ecc.size = ECC_SECTOR_SIZE;
chip->ecc.read_page = denali_read_page;
chip->ecc.read_page_raw = denali_read_page_raw;
chip->ecc.write_page = denali_write_page;
diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
index d621b74..5209625 100644
--- a/drivers/mtd/nand/denali.h
+++ b/drivers/mtd/nand/denali.h
@@ -396,8 +396,6 @@
#define MODE_10 0x08000000
#define MODE_11 0x0C000000
-#define ECC_SECTOR_SIZE 512
-
struct nand_buf {
int head;
int tail;
@@ -434,6 +432,7 @@ struct denali_nand_info {
#define DENALI_CAPS_HW_ECC_FIXUP BIT(0)
#define DENALI_CAPS_DMA_64BIT BIT(1)
#define DENALI_CAPS_NEW_N_BANKS_FORMAT BIT(2)
+#define DENALI_CAPS_ECC_SIZE_1024 BIT(3)
};
extern int denali_init(struct denali_nand_info *denali);
--
2.7.4
--
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 37/39] mtd: nand: denali: support "nand-ecc-strength" DT property
From: Masahiro Yamada @ 2016-11-26 18:06 UTC (permalink / raw)
To: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Masahiro Yamada, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Boris Brezillon, Marek Vasut,
Brian Norris, Richard Weinberger, David Woodhouse,
Cyrille Pitchen, Rob Herring, Mark Rutland
In-Reply-To: <1480183585-592-1-git-send-email-yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
Historically, this driver tried to choose as big ECC strength as
possible, but it would be reasonable to allow DT to set a particular
ECC strength with "nand-ecc-strength" property.
Going forward, DT platforms should specify "nand-ecc-strength" or
"nand-ecc-maximize" to show the ECC strength strategy explicitly.
If nothing is specified in DT, "nand-ecc-maximize" is implied since
this was the original behavior. It applies to PCI platforms too.
Signed-off-by: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
---
.../devicetree/bindings/mtd/denali-nand.txt | 5 ++++
drivers/mtd/nand/denali.c | 27 +++++++++++++++++++++-
drivers/mtd/nand/denali_pci.c | 2 ++
3 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/mtd/denali-nand.txt b/Documentation/devicetree/bindings/mtd/denali-nand.txt
index e9d5818..51fe195 100644
--- a/Documentation/devicetree/bindings/mtd/denali-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/denali-nand.txt
@@ -9,6 +9,11 @@ Required properties:
Optional properties:
- nand-ecc-step-size: must be 512 or 1024. If not specified, default to 512.
see nand.txt for details.
+ - nand-ecc-strength: see nand.txt for details
+ - nand-ecc-maximize: see nand.txt for details
+
+Note:
+Either nand-ecc-strength or nand-ecc-maximize should be specified.
The device tree may optionally contain sub-nodes describing partitions of the
address space. See partition.txt for more detail.
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 54c9e0c..df174ca 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1393,6 +1393,21 @@ static int denali_set_max_ecc_strength(struct denali_nand_info *denali)
return -EINVAL;
}
+static int denali_check_ecc_strength(struct denali_nand_info *denali)
+{
+ const int *ecc_strength = denali->ecc_strength_avail;
+
+ for (; *ecc_strength; ecc_strength++) {
+ if (*ecc_strength == denali->nand.ecc.strength)
+ return 0;
+ }
+
+ dev_err(denali->dev,
+ "Specified ECC strength is not supported for this controller.\n");
+
+ return -EINVAL;
+}
+
static int denali_ooblayout_ecc(struct mtd_info *mtd, int section,
struct mtd_oob_region *oobregion)
{
@@ -1628,7 +1643,17 @@ int denali_init(struct denali_nand_info *denali)
if (!denali->ecc_strength_avail)
denali->ecc_strength_avail = denali_default_ecc_strength;
- ret = denali_set_max_ecc_strength(denali);
+ if (!chip->ecc.strength && !(chip->ecc.options & NAND_ECC_MAXIMIZE)) {
+ dev_info(denali->dev,
+ "No ECC strength is specified. Trying max ECC strength strategy\n");
+ chip->ecc.options |= NAND_ECC_MAXIMIZE;
+ }
+
+ if (chip->ecc.options & NAND_ECC_MAXIMIZE)
+ ret = denali_set_max_ecc_strength(denali);
+ else
+ ret = denali_check_ecc_strength(denali);
+
if (ret)
goto failed_req_irq;
diff --git a/drivers/mtd/nand/denali_pci.c b/drivers/mtd/nand/denali_pci.c
index ac84323..0064f3e 100644
--- a/drivers/mtd/nand/denali_pci.c
+++ b/drivers/mtd/nand/denali_pci.c
@@ -85,6 +85,8 @@ static int denali_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
goto failed_remap_reg;
}
+ denali->nand.ecc.options |= NAND_ECC_MAXIMIZE;
+
ret = denali_init(denali);
if (ret)
goto failed_remap_mem;
--
2.7.4
--
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 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants
From: Masahiro Yamada @ 2016-11-26 18:06 UTC (permalink / raw)
To: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Masahiro Yamada, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Boris Brezillon, Marek Vasut,
Brian Norris, Richard Weinberger, David Woodhouse,
Cyrille Pitchen, Rob Herring, Mark Rutland
In-Reply-To: <1480183585-592-1-git-send-email-yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
Add two compatible strings for UniPhier SoCs. The revision register
on both shows revision 5.0, but they are different hardware.
Features:
- DMA engine with 64 bit physical address support
- 1024 byte ECC step size
- 8 / 16 / 24 bit ECC strength
- The n_banks format depends on SoC
Signed-off-by: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
---
.../devicetree/bindings/mtd/denali-nand.txt | 10 +++++--
drivers/mtd/nand/denali_dt.c | 33 ++++++++++++++++++++--
2 files changed, 38 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/denali-nand.txt b/Documentation/devicetree/bindings/mtd/denali-nand.txt
index 51fe195..cea46e2 100644
--- a/Documentation/devicetree/bindings/mtd/denali-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/denali-nand.txt
@@ -1,13 +1,19 @@
* Denali NAND controller
Required properties:
- - compatible : should be "denali,denali-nand-dt"
+ - compatible : should be one of the following:
+ "denali,denali-nand-dt"
+ "denali,denali-nand-uniphier-v5a"
+ "denali,denali-nand-uniphier-v5b"
- reg : should contain registers location and length for data and reg.
- reg-names: Should contain the reg names "nand_data" and "denali_reg"
- interrupts : The interrupt number.
Optional properties:
- - nand-ecc-step-size: must be 512 or 1024. If not specified, default to 512.
+ - nand-ecc-step-size: must be 512 or 1024. If not specified, default to:
+ 512 for "denali,denali-nand-dt"
+ 1024 for "denali,denali-nand-uniphier-v5a"
+ 1024 for "denali,denali-nand-uniphier-v5b"
see nand.txt for details.
- nand-ecc-strength: see nand.txt for details
- nand-ecc-maximize: see nand.txt for details
diff --git a/drivers/mtd/nand/denali_dt.c b/drivers/mtd/nand/denali_dt.c
index aa1e032..b411889 100644
--- a/drivers/mtd/nand/denali_dt.c
+++ b/drivers/mtd/nand/denali_dt.c
@@ -34,10 +34,37 @@ struct denali_dt_data {
unsigned int caps;
};
+static const int denali_uniphier_ecc_strength[] = {
+ 24, 16, 8, 0,
+};
+
+static const struct denali_dt_data denali_uniphier_v5a_data = {
+ .ecc_strength_avail = denali_uniphier_ecc_strength,
+ .caps = DENALI_CAPS_DMA_64BIT |
+ DENALI_CAPS_ECC_SIZE_1024,
+};
+
+static const struct denali_dt_data denali_uniphier_v5b_data = {
+ .ecc_strength_avail = denali_uniphier_ecc_strength,
+ .caps = DENALI_CAPS_DMA_64BIT |
+ DENALI_CAPS_NEW_N_BANKS_FORMAT |
+ DENALI_CAPS_ECC_SIZE_1024,
+};
+
static const struct of_device_id denali_nand_dt_ids[] = {
- { .compatible = "denali,denali-nand-dt" },
- { /* sentinel */ }
- };
+ {
+ .compatible = "denali,denali-nand-dt",
+ },
+ {
+ .compatible = "denali,denali-nand-uniphier-v5a",
+ .data = &denali_uniphier_v5a_data,
+ },
+ {
+ .compatible = "denali,denali-nand-uniphier-v5b",
+ .data = &denali_uniphier_v5b_data,
+ },
+ { /* sentinel */ }
+};
MODULE_DEVICE_TABLE(of, denali_nand_dt_ids);
--
2.7.4
--
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] arm64: dts: uniphier: add SD-ctrl node for LD11 SoC
From: Masahiro Yamada @ 2016-11-26 18:10 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Masahiro Yamada, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Will Deacon,
Mark Rutland, Catalin Marinas
The LD11 SoC is equipped with SD-ctrl (0x59810000) as well as
MIO-ctrl (0x5b3e0000). The SD-ctrl block on this SoC has just
one register for controlling RST_n pin of the eMMC device.
Signed-off-by: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
---
arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi b/arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi
index 0e5c58f..7a62fb9 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi
+++ b/arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi
@@ -273,6 +273,17 @@
reg = <0x59801000 0x400>;
};
+ sdctrl@59810000 {
+ compatible = "socionext,uniphier-ld11-sdctrl",
+ "simple-mfd", "syscon";
+ reg = <0x59810000 0x400>;
+
+ sd_rst: reset {
+ compatible = "socionext,uniphier-ld11-sd-reset";
+ #reset-cells = <1>;
+ };
+ };
+
perictrl@59820000 {
compatible = "socionext,uniphier-perictrl",
"simple-mfd", "syscon";
--
2.7.4
--
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 0/5] mfd: twl: improvements and new regulator driver
From: Nicolae Rosia @ 2016-11-26 18:13 UTC (permalink / raw)
To: Lee Jones, Mark Brown, Rob Herring, Mark Rutland, Tony Lindgren
Cc: Liam Girdwood, Paul Gortmaker, Graeme Gregory, Baruch Siach,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Nicolae Rosia
Hello,
The current TWL MFD driver has a number of problems which are
very well described by Russell King [0].
This series attemps to fix this by making the driver's private
structure available to child nodes.
A regulator driver for TWL6032 which makes use of the private
drvdata is introduced.
A driver for TWL6032 PMIC already exists in mainline,
twl-regulator, but it has the following drawbacks:
* has no mainline users
* it does not follow the recommended regulators binding since
it uses a compatible string for every regulator;
* it is broken
** the features flag is not set, hence the TWL6032
support is broken since it depends on TWL6032_SUBCLASS flag;
** even with that fixed, bit manipulations are wrong
If this receives positive feedback, I could convert all TWL drivers
to use drvdata, then get rid of the exported symbols.
[0] https://www.spinics.net/lists/linux-omap/msg133387.html
Nicolae Rosia (5):
mfd: twl-core: make driver DT only
mfd: twl: remove useless header
mfd: twl: move structure definitions to a public header
regulator: Add support for TI TWL6032
mfd: twl: use mfd_add_devices for TWL6032 regulator
.../bindings/regulator/twl6032-regulator.txt | 109 ++++
drivers/mfd/Kconfig | 1 +
drivers/mfd/twl-core.c | 444 ++--------------
drivers/mfd/twl-core.h | 10 -
drivers/mfd/twl4030-irq.c | 2 -
drivers/mfd/twl6030-irq.c | 2 -
drivers/regulator/Kconfig | 7 +
drivers/regulator/Makefile | 1 +
drivers/regulator/twl6032-regulator.c | 582 +++++++++++++++++++++
include/linux/mfd/twl-core.h | 35 ++
10 files changed, 768 insertions(+), 425 deletions(-)
create mode 100644 Documentation/devicetree/bindings/regulator/twl6032-regulator.txt
delete mode 100644 drivers/mfd/twl-core.h
create mode 100644 drivers/regulator/twl6032-regulator.c
create mode 100644 include/linux/mfd/twl-core.h
--
2.9.3
--
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 1/5] mfd: twl-core: make driver DT only
From: Nicolae Rosia @ 2016-11-26 18:13 UTC (permalink / raw)
To: Lee Jones, Mark Brown, Rob Herring, Mark Rutland, Tony Lindgren
Cc: Liam Girdwood, Paul Gortmaker, Graeme Gregory, Baruch Siach,
linux-omap, linux-arm-kernel, linux-kernel, devicetree,
Nicolae Rosia
In-Reply-To: <20161126181326.14951-1-Nicolae_Rosia@mentor.com>
All users are DT-only and it makes no sense to keep
unused code
Signed-off-by: Nicolae Rosia <Nicolae_Rosia@mentor.com>
---
drivers/mfd/Kconfig | 1 +
drivers/mfd/twl-core.c | 399 +------------------------------------------------
2 files changed, 8 insertions(+), 392 deletions(-)
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index c6df644..c180f8b 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1333,6 +1333,7 @@ config MFD_TPS80031
config TWL4030_CORE
bool "TI TWL4030/TWL5030/TWL6030/TPS659x0 Support"
depends on I2C=y
+ depends on OF
select IRQ_DOMAIN
select REGMAP_I2C
help
diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index c64615d..48b0668 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -13,6 +13,9 @@
* Code cleanup and modifications to IRQ handler.
* by syed khasim <x0khasim@ti.com>
*
+ * Code cleanup and modifications:
+ * Copyright (C) 2016 Nicolae Rosia <nicolae.rosia@gmail.com>
+ *
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
@@ -604,376 +607,6 @@ int twl_get_hfclk_rate(void)
}
EXPORT_SYMBOL_GPL(twl_get_hfclk_rate);
-static struct device *
-add_numbered_child(unsigned mod_no, const char *name, int num,
- void *pdata, unsigned pdata_len,
- bool can_wakeup, int irq0, int irq1)
-{
- struct platform_device *pdev;
- struct twl_client *twl;
- int status, sid;
-
- if (unlikely(mod_no >= twl_get_last_module())) {
- pr_err("%s: invalid module number %d\n", DRIVER_NAME, mod_no);
- return ERR_PTR(-EPERM);
- }
- sid = twl_priv->twl_map[mod_no].sid;
- twl = &twl_priv->twl_modules[sid];
-
- pdev = platform_device_alloc(name, num);
- if (!pdev)
- return ERR_PTR(-ENOMEM);
-
- pdev->dev.parent = &twl->client->dev;
-
- if (pdata) {
- status = platform_device_add_data(pdev, pdata, pdata_len);
- if (status < 0) {
- dev_dbg(&pdev->dev, "can't add platform_data\n");
- goto put_device;
- }
- }
-
- if (irq0) {
- struct resource r[2] = {
- { .start = irq0, .flags = IORESOURCE_IRQ, },
- { .start = irq1, .flags = IORESOURCE_IRQ, },
- };
-
- status = platform_device_add_resources(pdev, r, irq1 ? 2 : 1);
- if (status < 0) {
- dev_dbg(&pdev->dev, "can't add irqs\n");
- goto put_device;
- }
- }
-
- status = platform_device_add(pdev);
- if (status)
- goto put_device;
-
- device_init_wakeup(&pdev->dev, can_wakeup);
-
- return &pdev->dev;
-
-put_device:
- platform_device_put(pdev);
- dev_err(&twl->client->dev, "failed to add device %s\n", name);
- return ERR_PTR(status);
-}
-
-static inline struct device *add_child(unsigned mod_no, const char *name,
- void *pdata, unsigned pdata_len,
- bool can_wakeup, int irq0, int irq1)
-{
- return add_numbered_child(mod_no, name, -1, pdata, pdata_len,
- can_wakeup, irq0, irq1);
-}
-
-static struct device *
-add_regulator_linked(int num, struct regulator_init_data *pdata,
- struct regulator_consumer_supply *consumers,
- unsigned num_consumers, unsigned long features)
-{
- struct twl_regulator_driver_data drv_data;
-
- /* regulator framework demands init_data ... */
- if (!pdata)
- return NULL;
-
- if (consumers) {
- pdata->consumer_supplies = consumers;
- pdata->num_consumer_supplies = num_consumers;
- }
-
- if (pdata->driver_data) {
- /* If we have existing drv_data, just add the flags */
- struct twl_regulator_driver_data *tmp;
- tmp = pdata->driver_data;
- tmp->features |= features;
- } else {
- /* add new driver data struct, used only during init */
- drv_data.features = features;
- drv_data.set_voltage = NULL;
- drv_data.get_voltage = NULL;
- drv_data.data = NULL;
- pdata->driver_data = &drv_data;
- }
-
- /* NOTE: we currently ignore regulator IRQs, e.g. for short circuits */
- return add_numbered_child(TWL_MODULE_PM_MASTER, "twl_reg", num,
- pdata, sizeof(*pdata), false, 0, 0);
-}
-
-static struct device *
-add_regulator(int num, struct regulator_init_data *pdata,
- unsigned long features)
-{
- return add_regulator_linked(num, pdata, NULL, 0, features);
-}
-
-/*
- * NOTE: We know the first 8 IRQs after pdata->base_irq are
- * for the PIH, and the next are for the PWR_INT SIH, since
- * that's how twl_init_irq() sets things up.
- */
-
-static int
-add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
- unsigned long features)
-{
- struct device *child;
-
- if (IS_ENABLED(CONFIG_GPIO_TWL4030) && pdata->gpio) {
- child = add_child(TWL4030_MODULE_GPIO, "twl4030_gpio",
- pdata->gpio, sizeof(*pdata->gpio),
- false, irq_base + GPIO_INTR_OFFSET, 0);
- if (IS_ERR(child))
- return PTR_ERR(child);
- }
-
- if (IS_ENABLED(CONFIG_KEYBOARD_TWL4030) && pdata->keypad) {
- child = add_child(TWL4030_MODULE_KEYPAD, "twl4030_keypad",
- pdata->keypad, sizeof(*pdata->keypad),
- true, irq_base + KEYPAD_INTR_OFFSET, 0);
- if (IS_ERR(child))
- return PTR_ERR(child);
- }
-
- if (IS_ENABLED(CONFIG_TWL4030_MADC) && pdata->madc &&
- twl_class_is_4030()) {
- child = add_child(TWL4030_MODULE_MADC, "twl4030_madc",
- pdata->madc, sizeof(*pdata->madc),
- true, irq_base + MADC_INTR_OFFSET, 0);
- if (IS_ERR(child))
- return PTR_ERR(child);
- }
-
- if (IS_ENABLED(CONFIG_RTC_DRV_TWL4030)) {
- /*
- * REVISIT platform_data here currently might expose the
- * "msecure" line ... but for now we just expect board
- * setup to tell the chip "it's always ok to SET_TIME".
- * Eventually, Linux might become more aware of such
- * HW security concerns, and "least privilege".
- */
- child = add_child(TWL_MODULE_RTC, "twl_rtc", NULL, 0,
- true, irq_base + RTC_INTR_OFFSET, 0);
- if (IS_ERR(child))
- return PTR_ERR(child);
- }
-
- if (IS_ENABLED(CONFIG_PWM_TWL)) {
- child = add_child(TWL_MODULE_PWM, "twl-pwm", NULL, 0,
- false, 0, 0);
- if (IS_ERR(child))
- return PTR_ERR(child);
- }
-
- if (IS_ENABLED(CONFIG_PWM_TWL_LED)) {
- child = add_child(TWL_MODULE_LED, "twl-pwmled", NULL, 0,
- false, 0, 0);
- if (IS_ERR(child))
- return PTR_ERR(child);
- }
-
- if (IS_ENABLED(CONFIG_TWL4030_USB) && pdata->usb &&
- twl_class_is_4030()) {
-
- static struct regulator_consumer_supply usb1v5 = {
- .supply = "usb1v5",
- };
- static struct regulator_consumer_supply usb1v8 = {
- .supply = "usb1v8",
- };
- static struct regulator_consumer_supply usb3v1 = {
- .supply = "usb3v1",
- };
-
- /* First add the regulators so that they can be used by transceiver */
- if (IS_ENABLED(CONFIG_REGULATOR_TWL4030)) {
- /* this is a template that gets copied */
- struct regulator_init_data usb_fixed = {
- .constraints.valid_modes_mask =
- REGULATOR_MODE_NORMAL
- | REGULATOR_MODE_STANDBY,
- .constraints.valid_ops_mask =
- REGULATOR_CHANGE_MODE
- | REGULATOR_CHANGE_STATUS,
- };
-
- child = add_regulator_linked(TWL4030_REG_VUSB1V5,
- &usb_fixed, &usb1v5, 1,
- features);
- if (IS_ERR(child))
- return PTR_ERR(child);
-
- child = add_regulator_linked(TWL4030_REG_VUSB1V8,
- &usb_fixed, &usb1v8, 1,
- features);
- if (IS_ERR(child))
- return PTR_ERR(child);
-
- child = add_regulator_linked(TWL4030_REG_VUSB3V1,
- &usb_fixed, &usb3v1, 1,
- features);
- if (IS_ERR(child))
- return PTR_ERR(child);
-
- }
-
- child = add_child(TWL_MODULE_USB, "twl4030_usb",
- pdata->usb, sizeof(*pdata->usb), true,
- /* irq0 = USB_PRES, irq1 = USB */
- irq_base + USB_PRES_INTR_OFFSET,
- irq_base + USB_INTR_OFFSET);
-
- if (IS_ERR(child))
- return PTR_ERR(child);
-
- /* we need to connect regulators to this transceiver */
- if (IS_ENABLED(CONFIG_REGULATOR_TWL4030) && child) {
- usb1v5.dev_name = dev_name(child);
- usb1v8.dev_name = dev_name(child);
- usb3v1.dev_name = dev_name(child);
- }
- }
-
- if (IS_ENABLED(CONFIG_TWL4030_WATCHDOG) && twl_class_is_4030()) {
- child = add_child(TWL_MODULE_PM_RECEIVER, "twl4030_wdt", NULL,
- 0, false, 0, 0);
- if (IS_ERR(child))
- return PTR_ERR(child);
- }
-
- if (IS_ENABLED(CONFIG_INPUT_TWL4030_PWRBUTTON) && twl_class_is_4030()) {
- child = add_child(TWL_MODULE_PM_MASTER, "twl4030_pwrbutton",
- NULL, 0, true, irq_base + 8 + 0, 0);
- if (IS_ERR(child))
- return PTR_ERR(child);
- }
-
- if (IS_ENABLED(CONFIG_MFD_TWL4030_AUDIO) && pdata->audio &&
- twl_class_is_4030()) {
- child = add_child(TWL4030_MODULE_AUDIO_VOICE, "twl4030-audio",
- pdata->audio, sizeof(*pdata->audio),
- false, 0, 0);
- if (IS_ERR(child))
- return PTR_ERR(child);
- }
-
- /* twl4030 regulators */
- if (IS_ENABLED(CONFIG_REGULATOR_TWL4030) && twl_class_is_4030()) {
- child = add_regulator(TWL4030_REG_VPLL1, pdata->vpll1,
- features);
- if (IS_ERR(child))
- return PTR_ERR(child);
-
- child = add_regulator(TWL4030_REG_VIO, pdata->vio,
- features);
- if (IS_ERR(child))
- return PTR_ERR(child);
-
- child = add_regulator(TWL4030_REG_VDD1, pdata->vdd1,
- features);
- if (IS_ERR(child))
- return PTR_ERR(child);
-
- child = add_regulator(TWL4030_REG_VDD2, pdata->vdd2,
- features);
- if (IS_ERR(child))
- return PTR_ERR(child);
-
- child = add_regulator(TWL4030_REG_VMMC1, pdata->vmmc1,
- features);
- if (IS_ERR(child))
- return PTR_ERR(child);
-
- child = add_regulator(TWL4030_REG_VDAC, pdata->vdac,
- features);
- if (IS_ERR(child))
- return PTR_ERR(child);
-
- child = add_regulator((features & TWL4030_VAUX2)
- ? TWL4030_REG_VAUX2_4030
- : TWL4030_REG_VAUX2,
- pdata->vaux2, features);
- if (IS_ERR(child))
- return PTR_ERR(child);
-
- child = add_regulator(TWL4030_REG_VINTANA1, pdata->vintana1,
- features);
- if (IS_ERR(child))
- return PTR_ERR(child);
-
- child = add_regulator(TWL4030_REG_VINTANA2, pdata->vintana2,
- features);
- if (IS_ERR(child))
- return PTR_ERR(child);
-
- child = add_regulator(TWL4030_REG_VINTDIG, pdata->vintdig,
- features);
- if (IS_ERR(child))
- return PTR_ERR(child);
- }
-
- /* maybe add LDOs that are omitted on cost-reduced parts */
- if (IS_ENABLED(CONFIG_REGULATOR_TWL4030) && !(features & TPS_SUBSET)
- && twl_class_is_4030()) {
- child = add_regulator(TWL4030_REG_VPLL2, pdata->vpll2,
- features);
- if (IS_ERR(child))
- return PTR_ERR(child);
-
- child = add_regulator(TWL4030_REG_VMMC2, pdata->vmmc2,
- features);
- if (IS_ERR(child))
- return PTR_ERR(child);
-
- child = add_regulator(TWL4030_REG_VSIM, pdata->vsim,
- features);
- if (IS_ERR(child))
- return PTR_ERR(child);
-
- child = add_regulator(TWL4030_REG_VAUX1, pdata->vaux1,
- features);
- if (IS_ERR(child))
- return PTR_ERR(child);
-
- child = add_regulator(TWL4030_REG_VAUX3, pdata->vaux3,
- features);
- if (IS_ERR(child))
- return PTR_ERR(child);
-
- child = add_regulator(TWL4030_REG_VAUX4, pdata->vaux4,
- features);
- if (IS_ERR(child))
- return PTR_ERR(child);
- }
-
- if (IS_ENABLED(CONFIG_CHARGER_TWL4030) && pdata->bci &&
- !(features & (TPS_SUBSET | TWL5031))) {
- child = add_child(TWL_MODULE_MAIN_CHARGE, "twl4030_bci",
- pdata->bci, sizeof(*pdata->bci), false,
- /* irq0 = CHG_PRES, irq1 = BCI */
- irq_base + BCI_PRES_INTR_OFFSET,
- irq_base + BCI_INTR_OFFSET);
- if (IS_ERR(child))
- return PTR_ERR(child);
- }
-
- if (IS_ENABLED(CONFIG_TWL4030_POWER) && pdata->power) {
- child = add_child(TWL_MODULE_PM_MASTER, "twl4030_power",
- pdata->power, sizeof(*pdata->power), false,
- 0, 0);
- if (IS_ERR(child))
- return PTR_ERR(child);
- }
-
- return 0;
-}
-
-/*----------------------------------------------------------------------*/
-
/*
* These three functions initialize the on-chip clock framework,
* letting it generate the right frequencies for USB, MADC, and
@@ -1000,8 +633,7 @@ static inline int __init unprotect_pm_master(void)
return e;
}
-static void clocks_init(struct device *dev,
- struct twl4030_clock_init_data *clock)
+static void clocks_init(struct device *dev)
{
int e = 0;
struct clk *osc;
@@ -1031,8 +663,6 @@ static void clocks_init(struct device *dev,
}
ctrl |= HIGH_PERF_SQ;
- if (clock && clock->ck32k_lowpwr_enable)
- ctrl |= CK32K_LOWPWR_EN;
e |= unprotect_pm_master();
/* effect->MADC+USB ck en */
@@ -1071,16 +701,10 @@ static int twl_remove(struct i2c_client *client)
return 0;
}
-static struct of_dev_auxdata twl_auxdata_lookup[] = {
- OF_DEV_AUXDATA("ti,twl4030-gpio", 0, "twl4030-gpio", NULL),
- { /* sentinel */ },
-};
-
/* NOTE: This driver only handles a single twl4030/tps659x0 chip */
static int
twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
{
- struct twl4030_platform_data *pdata = dev_get_platdata(&client->dev);
struct device_node *node = client->dev.of_node;
struct platform_device *pdev;
const struct regmap_config *twl_regmap_config;
@@ -1088,8 +712,8 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
int status;
unsigned i, num_slaves;
- if (!node && !pdata) {
- dev_err(&client->dev, "no platform data\n");
+ if (!node) {
+ dev_err(&client->dev, "no DT info\n");
return -EINVAL;
}
@@ -1177,7 +801,7 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
twl_priv->ready = true;
/* setup clock framework */
- clocks_init(&pdev->dev, pdata ? pdata->clock : NULL);
+ clocks_init(&pdev->dev);
/* read TWL IDCODE Register */
if (twl_class_is_4030()) {
@@ -1225,15 +849,6 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
TWL4030_DCDC_GLOBAL_CFG);
}
- if (node) {
- if (pdata)
- twl_auxdata_lookup[0].platform_data = pdata->gpio;
- status = of_platform_populate(node, NULL, twl_auxdata_lookup,
- &client->dev);
- } else {
- status = add_children(pdata, irq_base, id->driver_data);
- }
-
fail:
if (status < 0)
twl_remove(client);
--
2.9.3
^ permalink raw reply related
* [PATCH 2/5] mfd: twl: remove useless header
From: Nicolae Rosia @ 2016-11-26 18:13 UTC (permalink / raw)
To: Lee Jones, Mark Brown, Rob Herring, Mark Rutland, Tony Lindgren
Cc: Liam Girdwood, Paul Gortmaker, Graeme Gregory, Baruch Siach,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Nicolae Rosia
In-Reply-To: <20161126181326.14951-1-Nicolae_Rosia-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
This header has one user, twl-core.c .
Remove it before it gets more users.
Signed-off-by: Nicolae Rosia <Nicolae_Rosia-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
---
drivers/mfd/twl-core.c | 8 ++++++--
drivers/mfd/twl-core.h | 10 ----------
drivers/mfd/twl4030-irq.c | 2 --
drivers/mfd/twl6030-irq.c | 2 --
4 files changed, 6 insertions(+), 16 deletions(-)
delete mode 100644 drivers/mfd/twl-core.h
diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 48b0668..e16084e 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -52,8 +52,6 @@
/* Register descriptions for audio */
#include <linux/mfd/twl4030-audio.h>
-#include "twl-core.h"
-
/*
* The TWL4030 "Triton 2" is one of a family of a multi-function "Power
* Management and System Companion Device" chips originally designed for
@@ -150,6 +148,12 @@
/*----------------------------------------------------------------------*/
+int twl6030_init_irq(struct device *dev, int irq_num);
+int twl6030_exit_irq(void);
+int twl4030_init_irq(struct device *dev, int irq_num);
+int twl4030_exit_irq(void);
+int twl4030_init_chip_irq(const char *chip);
+
/* Structure for each TWL4030/TWL6030 Slave */
struct twl_client {
struct i2c_client *client;
diff --git a/drivers/mfd/twl-core.h b/drivers/mfd/twl-core.h
deleted file mode 100644
index 6ff99dc..0000000
--- a/drivers/mfd/twl-core.h
+++ /dev/null
@@ -1,10 +0,0 @@
-#ifndef __TWL_CORE_H__
-#define __TWL_CORE_H__
-
-extern int twl6030_init_irq(struct device *dev, int irq_num);
-extern int twl6030_exit_irq(void);
-extern int twl4030_init_irq(struct device *dev, int irq_num);
-extern int twl4030_exit_irq(void);
-extern int twl4030_init_chip_irq(const char *chip);
-
-#endif /* __TWL_CORE_H__ */
diff --git a/drivers/mfd/twl4030-irq.c b/drivers/mfd/twl4030-irq.c
index b46c0cf..000c231 100644
--- a/drivers/mfd/twl4030-irq.c
+++ b/drivers/mfd/twl4030-irq.c
@@ -35,8 +35,6 @@
#include <linux/irqdomain.h>
#include <linux/i2c/twl.h>
-#include "twl-core.h"
-
/*
* TWL4030 IRQ handling has two stages in hardware, and thus in software.
* The Primary Interrupt Handler (PIH) stage exposes status bits saying
diff --git a/drivers/mfd/twl6030-irq.c b/drivers/mfd/twl6030-irq.c
index 5357450..63eca76 100644
--- a/drivers/mfd/twl6030-irq.c
+++ b/drivers/mfd/twl6030-irq.c
@@ -42,8 +42,6 @@
#include <linux/irqdomain.h>
#include <linux/of_device.h>
-#include "twl-core.h"
-
/*
* TWL6030 (unlike its predecessors, which had two level interrupt handling)
* three interrupt registers INT_STS_A, INT_STS_B and INT_STS_C.
--
2.9.3
--
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 3/5] mfd: twl: move structure definitions to a public header
From: Nicolae Rosia @ 2016-11-26 18:13 UTC (permalink / raw)
To: Lee Jones, Mark Brown, Rob Herring, Mark Rutland, Tony Lindgren
Cc: Liam Girdwood, Paul Gortmaker, Graeme Gregory, Baruch Siach,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Nicolae Rosia
In-Reply-To: <20161126181326.14951-1-Nicolae_Rosia-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
We want to get rid of exported symbols and have
the child devices use structure members directly.
Move the structure definitions to header and set
drvdata so child devices can access it.
Signed-off-by: Nicolae Rosia <Nicolae_Rosia-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
---
drivers/mfd/twl-core.c | 27 ++++-----------------------
include/linux/mfd/twl-core.h | 35 +++++++++++++++++++++++++++++++++++
2 files changed, 39 insertions(+), 23 deletions(-)
create mode 100644 include/linux/mfd/twl-core.h
diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index e16084e..409b836 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -48,6 +48,7 @@
#include <linux/i2c.h>
#include <linux/i2c/twl.h>
+#include <linux/mfd/twl-core.h>
/* Register descriptions for audio */
#include <linux/mfd/twl4030-audio.h>
@@ -154,28 +155,7 @@ int twl4030_init_irq(struct device *dev, int irq_num);
int twl4030_exit_irq(void);
int twl4030_init_chip_irq(const char *chip);
-/* Structure for each TWL4030/TWL6030 Slave */
-struct twl_client {
- struct i2c_client *client;
- struct regmap *regmap;
-};
-
-/* mapping the module id to slave id and base address */
-struct twl_mapping {
- unsigned char sid; /* Slave ID */
- unsigned char base; /* base address */
-};
-
-struct twl_private {
- bool ready; /* The core driver is ready to be used */
- u32 twl_idcode; /* TWL IDCODE Register value */
- unsigned int twl_id;
-
- struct twl_mapping *twl_map;
- struct twl_client *twl_modules;
-};
-
-static struct twl_private *twl_priv;
+static struct twlcore *twl_priv;
static struct twl_mapping twl4030_map[] = {
/*
@@ -745,7 +725,7 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
goto free;
}
- twl_priv = devm_kzalloc(&client->dev, sizeof(struct twl_private),
+ twl_priv = devm_kzalloc(&client->dev, sizeof(struct twlcore),
GFP_KERNEL);
if (!twl_priv) {
status = -ENOMEM;
@@ -803,6 +783,7 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
}
twl_priv->ready = true;
+ dev_set_drvdata(&client->dev, twl_priv);
/* setup clock framework */
clocks_init(&pdev->dev);
diff --git a/include/linux/mfd/twl-core.h b/include/linux/mfd/twl-core.h
new file mode 100644
index 0000000..d1c01b3
--- /dev/null
+++ b/include/linux/mfd/twl-core.h
@@ -0,0 +1,35 @@
+/*
+ * MFD core driver for the Texas Instruments TWL PMIC family
+ *
+ * Copyright (C) 2016 Nicolae Rosia <nicolae.rosia-Re5JQEeQqe8AvxtiuMwx3w@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 version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __TWL_CORE_H__
+#define __TWL_CORE_H__
+
+/* Structure for each TWL4030/TWL6030 Slave */
+struct twl_client {
+ struct i2c_client *client;
+ struct regmap *regmap;
+};
+
+/* mapping the module id to slave id and base address */
+struct twl_mapping {
+ unsigned char sid; /* Slave ID */
+ unsigned char base; /* base address */
+};
+
+struct twlcore {
+ bool ready; /* The core driver is ready to be used */
+ u32 twl_idcode; /* TWL IDCODE Register value */
+ unsigned int twl_id;
+
+ struct twl_mapping *twl_map;
+ struct twl_client *twl_modules;
+};
+
+#endif
--
2.9.3
--
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 4/5] regulator: Add support for TI TWL6032
From: Nicolae Rosia @ 2016-11-26 18:13 UTC (permalink / raw)
To: Lee Jones, Mark Brown, Rob Herring, Mark Rutland, Tony Lindgren
Cc: Liam Girdwood, Paul Gortmaker, Graeme Gregory, Baruch Siach,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Nicolae Rosia
In-Reply-To: <20161126181326.14951-1-Nicolae_Rosia-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
The TWL6032 PMIC is similar to TWL6030, has different
output names, and regulator control logic.
It is used on Barnes & Noble Nook HD and HD+.
Signed-off-by: Nicolae Rosia <Nicolae_Rosia-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
---
.../bindings/regulator/twl6032-regulator.txt | 109 ++++
drivers/regulator/Kconfig | 7 +
drivers/regulator/Makefile | 1 +
drivers/regulator/twl6032-regulator.c | 582 +++++++++++++++++++++
4 files changed, 699 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/twl6032-regulator.txt
create mode 100644 drivers/regulator/twl6032-regulator.c
diff --git a/Documentation/devicetree/bindings/regulator/twl6032-regulator.txt b/Documentation/devicetree/bindings/regulator/twl6032-regulator.txt
new file mode 100644
index 0000000..323f5a9
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/twl6032-regulator.txt
@@ -0,0 +1,109 @@
+TWL6032 PMIC Voltage Regulator Bindings
+
+The parent node must be MFD TWL Core, ti,twl6032.
+
+Required properties:
+- compatible: "ti,twl6032"
+
+Optional properties:
+- regulators node containing regulator childs.
+
+The child regulators must be named after their hardware
+counterparts: LDO[1-6], LDOLN, LDOUSB and VANA.
+
+Each regulator is defined using the standard binding
+for regulators as described in ./regulator.txt
+
+Example:
+twl {
+ compatible = "ti,twl6032";
+
+ [...]
+
+ pmic {
+ compatible = "ti,twl6032-regulator";
+
+ regulators {
+ ldo1: LDO1 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <2500000>;
+
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
+ };
+
+ ldo2: LDO2 {
+ regulator-min-microvolt = <1000000>;
+ regulator-max-microvolt = <3000000>;
+
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
+ };
+
+ ldo3: LDO3 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-boot-on;
+
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
+ };
+
+ ldo4: LDO4 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
+ };
+
+ ldo5: LDO5 {
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <3000000>;
+
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
+ };
+
+ ldo6: LDO6 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-always-on;
+
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
+ };
+
+ ldo7: LDO7 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ ldoln: LDOLN {
+ regulator-min-microvolt = <1000000>;
+ regulator-max-microvolt = <3000000>;
+ };
+
+ ldousb: LDOUSB {
+ regulator-min-microvolt = <1000000>;
+ regulator-max-microvolt = <3000000>;
+ };
+
+ vana: VANA {
+ regulator-min-microvolt = <2100000>;
+ regulator-max-microvolt = <2100000>;
+ regulator-always-on;
+ };
+ };
+ };
+
+ [...]
+};
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 936f7cc..3168aba 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -843,6 +843,13 @@ config REGULATOR_TWL4030
This driver supports the voltage regulators provided by
this family of companion chips.
+config REGULATOR_TWL6032
+ tristate "TI TWL6032 PMIC"
+ depends on TWL4030_CORE
+ depends on OF || COMPILE_TEST
+ help
+ This driver supports the Texas Instruments TWL6032 voltage regulator.
+
config REGULATOR_VEXPRESS
tristate "Versatile Express regulators"
depends on VEXPRESS_CONFIG
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 2142a5d..185a979 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -105,6 +105,7 @@ obj-$(CONFIG_REGULATOR_TPS65910) += tps65910-regulator.o
obj-$(CONFIG_REGULATOR_TPS65912) += tps65912-regulator.o
obj-$(CONFIG_REGULATOR_TPS80031) += tps80031-regulator.o
obj-$(CONFIG_REGULATOR_TWL4030) += twl-regulator.o
+obj-$(CONFIG_REGULATOR_TWL6032) += twl6032-regulator.o
obj-$(CONFIG_REGULATOR_VEXPRESS) += vexpress-regulator.o
obj-$(CONFIG_REGULATOR_WM831X) += wm831x-dcdc.o
obj-$(CONFIG_REGULATOR_WM831X) += wm831x-isink.o
diff --git a/drivers/regulator/twl6032-regulator.c b/drivers/regulator/twl6032-regulator.c
new file mode 100644
index 0000000..70a0fdf
--- /dev/null
+++ b/drivers/regulator/twl6032-regulator.c
@@ -0,0 +1,582 @@
+/*
+ * TWL6032 regulator driver
+ * Copyright (C) 2016 Nicolae Rosia <nicolae.rosia-Re5JQEeQqe8AvxtiuMwx3w@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.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/irq.h>
+#include <linux/mfd/twl-core.h>
+
+/* TWL6032 register offsets */
+#define TWL6032_VREG_TRANS 1
+#define TWL6032_VREG_STATE 2
+#define TWL6032_VREG_VOLTAGE 3
+
+#define TWL6032_LDO_MIN_MV 1000
+#define TWL6032_LDO_MAX_MV 3300
+
+/* TWL6030 LDO register values for CFG_TRANS */
+#define TWL6032_CFG_TRANS_STATE_MASK 0x03
+#define TWL6032_CFG_TRANS_STATE_OFF 0x00
+#define TWL6032_CFG_TRANS_STATE_AUTO 0x01
+#define TWL6032_CFG_TRANS_SLEEP_SHIFT 2
+
+#define TWL6032_CFG_STATE_MASK 0x03
+#define TWL6032_CFG_STATE_OFF 0x00
+#define TWL6032_CFG_STATE_ON 0x01
+#define TWL6032_CFG_STATE_OFF2 0x02
+#define TWL6032_CFG_STATE_SLEEP 0x03
+
+static const char *rdev_get_name(struct regulator_dev *rdev)
+{
+ if (rdev->constraints && rdev->constraints->name)
+ return rdev->constraints->name;
+ else if (rdev->desc->name)
+ return rdev->desc->name;
+ else
+ return "";
+}
+
+struct twl6032_regulator_info {
+ u8 base;
+ unsigned int min_mV;
+ struct regulator_desc desc;
+};
+
+struct twl6032_regulator {
+ struct twl6032_regulator_info *info;
+};
+
+static int twl6032_set_trans_state(struct regulator_dev *rdev, u8 shift, u8 val)
+{
+ struct twl6032_regulator *twl6032_reg = rdev_get_drvdata(rdev);
+ struct twl6032_regulator_info *info = twl6032_reg->info;
+ unsigned int state;
+ u8 mask;
+ int ret;
+
+ /* Read CFG_TRANS register of TWL6030 */
+ ret = regmap_read(rdev->regmap, info->base + TWL6032_VREG_TRANS,
+ &state);
+ if (ret < 0) {
+ dev_err(&rdev->dev, "%s %s: regmap_read: %d\n",
+ rdev_get_name(rdev), __func__, ret);
+ return ret;
+ }
+
+ mask = TWL6032_CFG_TRANS_STATE_MASK << shift;
+ val = (val << shift) & mask;
+
+ /* If value is already set, no need to write to reg */
+ if (val == (state & mask))
+ return 0;
+
+ state &= ~mask;
+ state |= val;
+
+ return regmap_write(rdev->regmap, info->base + TWL6032_VREG_TRANS,
+ state);
+}
+
+static int
+twl6032_ldo_list_voltage(struct regulator_dev *rdev, unsigned int sel)
+{
+ int ret;
+
+ switch (sel) {
+ case 0:
+ ret = 0;
+ break;
+ case 1 ... 24:
+ /* Linear mapping from 00000001 to 00011000:
+ * Absolute voltage value = 1.0 V + 0.1 V × (sel – 00000001)
+ */
+ ret = (TWL6032_LDO_MIN_MV + 100 * (sel - 1)) * 1000;
+ break;
+ case 25 ... 30:
+ ret = -EINVAL;
+ break;
+ case 31:
+ ret = 2750000;
+ break;
+ default:
+ ret = -EINVAL;
+ }
+
+ dev_dbg(&rdev->dev, "%s %s: sel: %d, mV: %d\n", rdev_get_name(rdev),
+ __func__, sel, ret);
+
+ return ret;
+}
+
+static int
+twl6032_ldo_set_voltage_sel(struct regulator_dev *rdev, unsigned int sel)
+{
+ struct twl6032_regulator *twl6032_reg = rdev_get_drvdata(rdev);
+ struct twl6032_regulator_info *info = twl6032_reg->info;
+ int ret;
+
+ dev_dbg(&rdev->dev, "%s %s: sel: 0x%02X\n", rdev_get_name(rdev),
+ __func__, sel);
+
+ ret = regmap_write(rdev->regmap, info->base + TWL6032_VREG_VOLTAGE,
+ sel);
+ if (ret < 0) {
+ dev_err(&rdev->dev, "%s %s: regmap_write: %d\n",
+ rdev_get_name(rdev), __func__, ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int twl6032_ldo_get_voltage_sel(struct regulator_dev *rdev)
+{
+ struct twl6032_regulator *twl6032_reg = rdev_get_drvdata(rdev);
+ struct twl6032_regulator_info *info = twl6032_reg->info;
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(rdev->regmap, info->base + TWL6032_VREG_VOLTAGE,
+ &val);
+ if (ret < 0) {
+ dev_err(&rdev->dev, "%s %s: regmap_read: %d\n",
+ rdev_get_name(rdev), __func__, ret);
+ return ret;
+ }
+
+ dev_dbg(&rdev->dev, "%s %s: vsel: 0x%02X\n", rdev_get_name(rdev),
+ __func__, val);
+
+ return val;
+}
+
+static int twl6032_ldo_enable(struct regulator_dev *rdev)
+{
+ struct twl6032_regulator *twl6032_reg = rdev_get_drvdata(rdev);
+ struct twl6032_regulator_info *info = twl6032_reg->info;
+ int ret;
+
+ dev_dbg(&rdev->dev, "%s %s\n", rdev_get_name(rdev), __func__);
+
+ ret = regmap_write(rdev->regmap, info->base + TWL6032_VREG_STATE,
+ TWL6032_CFG_STATE_ON);
+ if (ret < 0) {
+ dev_err(&rdev->dev, "%s %s: regmap_write: %d\n",
+ rdev_get_name(rdev), __func__, ret);
+ return ret;
+ }
+
+ ret = twl6032_set_trans_state(rdev, TWL6032_CFG_TRANS_SLEEP_SHIFT,
+ TWL6032_CFG_TRANS_STATE_AUTO);
+ if (ret < 0) {
+ dev_err(&rdev->dev, "%s %s: twl6032_set_trans_state: %d\n",
+ rdev_get_name(rdev), __func__, ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int twl6032_ldo_disable(struct regulator_dev *rdev)
+{
+ struct twl6032_regulator *twl6032_reg = rdev_get_drvdata(rdev);
+ struct twl6032_regulator_info *info = twl6032_reg->info;
+ int ret;
+
+ dev_dbg(&rdev->dev, "%s %s\n", rdev_get_name(rdev), __func__);
+
+ ret = regmap_write(rdev->regmap, info->base + TWL6032_VREG_STATE,
+ TWL6032_CFG_STATE_OFF);
+ if (ret < 0) {
+ dev_err(&rdev->dev, "%s %s: regmap_write: %d\n",
+ rdev_get_name(rdev), __func__, ret);
+ return ret;
+ }
+
+ ret = twl6032_set_trans_state(rdev, TWL6032_CFG_TRANS_SLEEP_SHIFT,
+ TWL6032_CFG_TRANS_STATE_OFF);
+ if (ret < 0) {
+ dev_err(&rdev->dev, "%s %s: twl6032_set_trans_state: %d\n",
+ rdev_get_name(rdev), __func__, ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int twl6032_ldo_is_enabled(struct regulator_dev *rdev)
+{
+ struct twl6032_regulator *twl6032_reg = rdev_get_drvdata(rdev);
+ struct twl6032_regulator_info *info = twl6032_reg->info;
+ int ret;
+ unsigned int val;
+
+ ret = regmap_read(rdev->regmap, info->base + TWL6032_VREG_STATE, &val);
+ if (ret < 0) {
+ dev_err(&rdev->dev, "%s regmap_read: %d\n", __func__, ret);
+ return ret;
+ }
+
+ dev_dbg(&rdev->dev, "%s %s: val: 0x%02X, val-masked: 0x%02X, ret: %d\n",
+ rdev_get_name(rdev), __func__,
+ val, val & TWL6032_CFG_STATE_MASK,
+ (val & TWL6032_CFG_STATE_MASK) == TWL6032_CFG_STATE_ON);
+
+ val &= TWL6032_CFG_STATE_MASK;
+
+ return val == TWL6032_CFG_STATE_ON;
+}
+
+static int twl6032_ldo_set_mode(struct regulator_dev *rdev, unsigned int mode)
+{
+ struct twl6032_regulator *twl6032_reg = rdev_get_drvdata(rdev);
+ struct twl6032_regulator_info *info = twl6032_reg->info;
+ unsigned int val = 0;
+ int ret;
+
+ dev_dbg(&rdev->dev, "%s %s: mode: 0x%02X\n", rdev_get_name(rdev),
+ __func__, mode);
+
+ switch (mode) {
+ case REGULATOR_MODE_NORMAL:
+ val |= TWL6032_CFG_STATE_ON;
+ break;
+ case REGULATOR_MODE_STANDBY:
+ val |= TWL6032_CFG_STATE_SLEEP;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ ret = regmap_write(rdev->regmap, info->base + TWL6032_VREG_STATE, val);
+ if (ret < 0) {
+ dev_err(&rdev->dev, "%s %s: regmap_write: %d\n",
+ rdev_get_name(rdev), __func__, ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int twl6032_ldo_get_status(struct regulator_dev *rdev)
+{
+ struct twl6032_regulator *twl6032_reg = rdev_get_drvdata(rdev);
+ struct twl6032_regulator_info *info = twl6032_reg->info;
+ int ret;
+ unsigned int val;
+
+ ret = regmap_read(rdev->regmap, info->base + TWL6032_VREG_STATE, &val);
+ if (ret < 0) {
+ dev_err(&rdev->dev, "%s %s: regmap_read: %d\n",
+ rdev_get_name(rdev), __func__, ret);
+ return ret;
+ }
+
+ dev_dbg(&rdev->dev, "%s %s: val: 0x%02X, val-with-mask: 0x%02X\n",
+ rdev_get_name(rdev), __func__,
+ val, val & TWL6032_CFG_STATE_MASK);
+
+ val &= TWL6032_CFG_STATE_MASK;
+
+ switch (val) {
+ case TWL6032_CFG_STATE_ON:
+ return REGULATOR_STATUS_NORMAL;
+
+ case TWL6032_CFG_STATE_SLEEP:
+ return REGULATOR_STATUS_STANDBY;
+
+ case TWL6032_CFG_STATE_OFF:
+ case TWL6032_CFG_STATE_OFF2:
+ default:
+ break;
+ }
+
+ return REGULATOR_STATUS_OFF;
+}
+
+static int twl6032_ldo_suspend_enable(struct regulator_dev *rdev)
+{
+ return twl6032_set_trans_state(rdev, TWL6032_CFG_TRANS_SLEEP_SHIFT,
+ TWL6032_CFG_TRANS_STATE_AUTO);
+}
+
+static int twl6032_ldo_suspend_disable(struct regulator_dev *rdev)
+{
+ return twl6032_set_trans_state(rdev, TWL6032_CFG_TRANS_SLEEP_SHIFT,
+ TWL6032_CFG_TRANS_STATE_OFF);
+}
+
+static int
+twl6032_fixed_list_voltage(struct regulator_dev *rdev, unsigned int sel)
+{
+ struct twl6032_regulator *twl6032_reg = rdev_get_drvdata(rdev);
+ struct twl6032_regulator_info *info = twl6032_reg->info;
+
+ return info->min_mV * 1000; /* mV to V */
+}
+
+static int twl6032_fixed_get_voltage(struct regulator_dev *rdev)
+{
+ struct twl6032_regulator *twl6032_reg = rdev_get_drvdata(rdev);
+ struct twl6032_regulator_info *info = twl6032_reg->info;
+
+ return info->min_mV * 1000; /* mV to V */
+}
+
+static const struct regulator_ops twl6032_ldo_ops = {
+ .list_voltage = twl6032_ldo_list_voltage,
+ .set_voltage_sel = twl6032_ldo_set_voltage_sel,
+ .get_voltage_sel = twl6032_ldo_get_voltage_sel,
+ .enable = twl6032_ldo_enable,
+ .disable = twl6032_ldo_disable,
+ .is_enabled = twl6032_ldo_is_enabled,
+ .set_mode = twl6032_ldo_set_mode,
+ .get_status = twl6032_ldo_get_status,
+ .set_suspend_enable = twl6032_ldo_suspend_enable,
+ .set_suspend_disable = twl6032_ldo_suspend_disable,
+};
+
+static const struct regulator_ops twl6032_fixed_ops = {
+ .list_voltage = twl6032_fixed_list_voltage,
+ .get_voltage = twl6032_fixed_get_voltage,
+ .enable = twl6032_ldo_enable,
+ .disable = twl6032_ldo_disable,
+ .is_enabled = twl6032_ldo_is_enabled,
+ .set_mode = twl6032_ldo_set_mode,
+ .get_status = twl6032_ldo_get_status,
+ .set_suspend_enable = twl6032_ldo_suspend_enable,
+ .set_suspend_disable = twl6032_ldo_suspend_disable,
+};
+
+#define TWL6032_LDO_REG_VOLTAGES \
+ ((TWL6032_LDO_MAX_MV - TWL6032_LDO_MIN_MV) / 100 + 1)
+#define TWL6032_LDO_REG(_id, _reg) \
+{ \
+ .base = _reg, \
+ .desc = { \
+ .name = "twl6032-reg-" # _id, \
+ .n_voltages = TWL6032_LDO_REG_VOLTAGES, \
+ .ops = &twl6032_ldo_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .owner = THIS_MODULE, \
+ }, \
+}
+
+#define TWL6032_FIXED_REG(_id, _reg, _min_mV) \
+{ \
+ .base = _reg, \
+ .min_mV = _min_mV, \
+ .desc = { \
+ .name = "twl6032-reg-" # _id, \
+ .n_voltages = 1, \
+ .ops = &twl6032_fixed_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .owner = THIS_MODULE, \
+ }, \
+}
+
+#define TWL6032_RESOURCE_REG(_id, _reg) \
+{ \
+ .base = _reg, \
+ .desc = { \
+ .name = "twl6032-reg-" # _id, \
+ .ops = &twl6032_ldo_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .owner = THIS_MODULE, \
+ }, \
+}
+
+static struct twl6032_regulator_info twl6032_ldo_reg_info[] = {
+ TWL6032_LDO_REG(LDO1, 0x9C),
+ TWL6032_LDO_REG(LDO2, 0x84),
+ TWL6032_LDO_REG(LDO3, 0x8C),
+ TWL6032_LDO_REG(LDO4, 0x88),
+ TWL6032_LDO_REG(LDO5, 0x98),
+ TWL6032_LDO_REG(LDO6, 0x90),
+ TWL6032_LDO_REG(LDO7, 0xA4),
+ TWL6032_LDO_REG(LDOLN, 0x94),
+ TWL6032_LDO_REG(LDOUSB, 0xA0),
+};
+
+static struct twl6032_regulator_info twl6032_fixed_reg_info[] = {
+ TWL6032_FIXED_REG(VANA, 0x80, 2100),
+};
+
+static struct of_regulator_match
+twl6032_ldo_reg_matches[] = {
+ { .name = "LDO1", },
+ { .name = "LDO2", },
+ { .name = "LDO3", },
+ { .name = "LDO4", },
+ { .name = "LDO5", },
+ { .name = "LDO6", },
+ { .name = "LDO7", },
+ { .name = "LDOLN" },
+ { .name = "LDOUSB" }
+};
+
+static struct of_regulator_match
+twl6032_fixed_reg_matches[] = {
+ { .name = "VANA", },
+};
+
+#define TWL6032_LDO_REG_NUM ARRAY_SIZE(twl6032_ldo_reg_matches)
+#define TWL6032_FIXED_REG_NUM ARRAY_SIZE(twl6032_fixed_reg_matches)
+
+struct twl6032_regulator_priv {
+ struct twl6032_regulator ldo_regulators[TWL6032_LDO_REG_NUM];
+ struct twl6032_regulator fixed_regulators[TWL6032_FIXED_REG_NUM];
+};
+
+static int twl6032_regulator_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *regulators;
+ struct of_regulator_match *match;
+ struct twlcore *twl = dev_get_drvdata(pdev->dev.parent);
+ struct twl6032_regulator_priv *priv;
+ struct regulator_config config = {
+ .dev = &pdev->dev,
+ };
+ struct regulator_dev *rdev;
+ int ret, i;
+
+ if (!dev->of_node) {
+ dev_err(&pdev->dev, "no DT info\n");
+ return -EINVAL;
+ }
+
+ regulators = of_get_child_by_name(dev->of_node, "regulators");
+ if (!regulators) {
+ dev_err(dev, "regulator node not found\n");
+ return -EINVAL;
+ }
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, priv);
+
+ /* LDO regulators parsing */
+ ret = of_regulator_match(dev, regulators, twl6032_ldo_reg_matches,
+ TWL6032_LDO_REG_NUM);
+ of_node_put(regulators);
+ if (ret < 0) {
+ dev_err(dev, "error parsing LDO reg init data: %d\n", ret);
+ return ret;
+ }
+
+ for (i = 0; i < TWL6032_LDO_REG_NUM; i++) {
+ struct twl6032_regulator *twl6032_reg;
+
+ match = &twl6032_ldo_reg_matches[i];
+ if (!match->of_node)
+ continue;
+
+ twl6032_reg = &priv->ldo_regulators[i];
+ twl6032_reg->info = &twl6032_ldo_reg_info[i];
+
+ config.init_data = match->init_data;
+ config.driver_data = &priv->ldo_regulators[i];
+ config.regmap = twl->twl_modules[0].regmap;
+ config.of_node = match->of_node;
+
+ rdev = devm_regulator_register(dev, &twl6032_reg->info->desc,
+ &config);
+ if (IS_ERR(rdev)) {
+ ret = PTR_ERR(rdev);
+ dev_err(dev, "failed to register regulator %s: %d\n",
+ twl6032_reg->info->desc.name, ret);
+ return ret;
+ }
+ }
+
+ /* Fixed regulators parsing */
+ ret = of_regulator_match(dev, regulators, twl6032_fixed_reg_matches,
+ TWL6032_FIXED_REG_NUM);
+ of_node_put(regulators);
+ if (ret < 0) {
+ dev_err(dev, "error parsing fixed reg init data: %d\n", ret);
+ return ret;
+ }
+
+ for (i = 0; i < TWL6032_FIXED_REG_NUM; i++) {
+ struct twl6032_regulator *twl6032_reg;
+
+ match = &twl6032_fixed_reg_matches[i];
+ if (!match->of_node)
+ continue;
+
+ twl6032_reg = &priv->fixed_regulators[i];
+ twl6032_reg->info = &twl6032_fixed_reg_info[i];
+
+ config.init_data = match->init_data;
+ config.driver_data = &priv->fixed_regulators[i];
+ config.regmap = twl->twl_modules[0].regmap;
+ config.of_node = match->of_node;
+
+ rdev = devm_regulator_register(dev, &twl6032_reg->info->desc,
+ &config);
+ if (IS_ERR(rdev)) {
+ ret = PTR_ERR(rdev);
+ dev_err(dev, "failed to register regulator %s: %d\n",
+ twl6032_reg->info->desc.name, ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static int twl6032_regulator_remove(struct platform_device *pdev)
+{
+ return 0;
+}
+
+static const struct of_device_id twl6032_dt_match[] = {
+ { .compatible = "ti,twl6032-regulator" },
+ { /* last entry */ }
+};
+
+MODULE_DEVICE_TABLE(platform, twl6032_regulator_driver_ids);
+
+static struct platform_driver twl6032_regulator_driver = {
+ .driver = {
+ .name = "twl6032-regulator",
+ .of_match_table = twl6032_dt_match,
+ },
+ .probe = twl6032_regulator_probe,
+ .remove = twl6032_regulator_remove,
+};
+
+static int __init twl6032_regulator_init(void)
+{
+ return platform_driver_register(&twl6032_regulator_driver);
+}
+subsys_initcall(twl6032_regulator_init);
+
+static void __exit twl6032_regulator_exit(void)
+{
+ platform_driver_unregister(&twl6032_regulator_driver);
+}
+module_exit(twl6032_regulator_exit);
+
+MODULE_AUTHOR("Nicolae Rosia <nicolae.rosia-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
+MODULE_DESCRIPTION("TI TWL6032 Regulator Driver");
+MODULE_LICENSE("GPL v2");
--
2.9.3
--
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 5/5] mfd: twl: use mfd_add_devices for TWL6032 regulator
From: Nicolae Rosia @ 2016-11-26 18:13 UTC (permalink / raw)
To: Lee Jones, Mark Brown, Rob Herring, Mark Rutland, Tony Lindgren
Cc: Liam Girdwood, Paul Gortmaker, Graeme Gregory, Baruch Siach,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Nicolae Rosia
In-Reply-To: <20161126181326.14951-1-Nicolae_Rosia-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
TWL6032 regulator driver uses the drvdata twl_priv pointer.
In order to avoid accessing an invalid drvdata
when the driver gets unbinded, make sure we remove the
child devices before deleting the drvdata.
Signed-off-by: Nicolae Rosia <Nicolae_Rosia-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
---
drivers/mfd/twl-core.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 409b836..1e94364 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -43,6 +43,7 @@
#include <linux/of_platform.h>
#include <linux/irq.h>
#include <linux/irqdomain.h>
+#include <linux/mfd/core.h>
#include <linux/regulator/machine.h>
@@ -155,8 +156,16 @@ int twl4030_init_irq(struct device *dev, int irq_num);
int twl4030_exit_irq(void);
int twl4030_init_chip_irq(const char *chip);
+
static struct twlcore *twl_priv;
+static struct mfd_cell twl6032_devs[] = {
+ {
+ .name = "twl6032-regulator",
+ .of_compatible = "ti,twl6032-regulator",
+ },
+};
+
static struct twl_mapping twl4030_map[] = {
/*
* NOTE: don't change this table without updating the
@@ -665,6 +674,8 @@ static int twl_remove(struct i2c_client *client)
unsigned i, num_slaves;
int status;
+ mfd_remove_devices(&client->dev);
+
if (twl_class_is_4030())
status = twl4030_exit_irq();
else
@@ -834,6 +845,17 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
TWL4030_DCDC_GLOBAL_CFG);
}
+ if (id->driver_data & TWL6032_SUBCLASS) {
+ status = mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
+ twl6032_devs, ARRAY_SIZE(twl6032_devs),
+ NULL, 0, NULL);
+ if (status != 0) {
+ dev_err(&client->dev, "failed to add mfd devices: %d\n",
+ status);
+ goto fail;
+ }
+ }
+
fail:
if (status < 0)
twl_remove(client);
--
2.9.3
--
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 v4 0/7] mux controller abstraction and iio/i2c muxes
From: Peter Rosin @ 2016-11-26 18:38 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Jonathan Cameron,
Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Jonathan Corbet, Arnd Bergmann, Greg Kroah-Hartman,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480000687-5630-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
On 2016-11-24 16:18, Peter Rosin wrote:
> v3 -> v4 changes
> - added support for having the mux-controller in a child node of a
> mux-consumer if it is a sole consumer, to hopefully even further satisfy
> the complaint from Rob (and later Lars-Peter) about dt complexity.
> - the above came at the cost of some rather horrible refcounting code,
> please review and suggest how it should be done...
>
> v2 -> v3 changes
> - have the mux-controller in the parent node of any mux-controller consumer,
> to hopefully satisfy complaint from Rob about dt complexity.
I did some further tests and both of these attempts to support fancier
devicetree bindings have severe problems. I will remove them for v5 and
go back to having a phandle reference to the mux-controller from the
consumer (unless I get some revelation of course and just get it). I'm
simply not yet understanding the driver model well enough to pull this
off at the moment...
Cheers,
Peter
--
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 4/5] regulator: Add support for TI TWL6032
From: kbuild test robot @ 2016-11-26 18:55 UTC (permalink / raw)
Cc: kbuild-all-JC7UmRfGjtg, Lee Jones, Mark Brown, Rob Herring,
Mark Rutland, Tony Lindgren, Liam Girdwood, Paul Gortmaker,
Graeme Gregory, Baruch Siach, linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Nicolae Rosia
In-Reply-To: <20161126181326.14951-5-Nicolae_Rosia-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2168 bytes --]
Hi Nicolae,
[auto build test ERROR on omap/for-next]
[also build test ERROR on v4.9-rc6]
[cannot apply to next-20161125]
[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/Nicolae-Rosia/mfd-twl-improvements-and-new-regulator-driver/20161127-022201
base: https://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git for-next
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
In file included from drivers/regulator/twl6032-regulator.c:11:0:
>> drivers/regulator/twl6032-regulator.c:557:31: error: 'twl6032_regulator_driver_ids' undeclared here (not in a function)
MODULE_DEVICE_TABLE(platform, twl6032_regulator_driver_ids);
^
include/linux/module.h:213:21: note: in definition of macro 'MODULE_DEVICE_TABLE'
extern const typeof(name) __mod_##type##__##name##_device_table \
^~~~
>> include/linux/module.h:213:27: error: '__mod_platform__twl6032_regulator_driver_ids_device_table' aliased to undefined symbol 'twl6032_regulator_driver_ids'
extern const typeof(name) __mod_##type##__##name##_device_table \
^
>> drivers/regulator/twl6032-regulator.c:557:1: note: in expansion of macro 'MODULE_DEVICE_TABLE'
MODULE_DEVICE_TABLE(platform, twl6032_regulator_driver_ids);
^~~~~~~~~~~~~~~~~~~
vim +/twl6032_regulator_driver_ids +557 drivers/regulator/twl6032-regulator.c
551
552 static const struct of_device_id twl6032_dt_match[] = {
553 { .compatible = "ti,twl6032-regulator" },
554 { /* last entry */ }
555 };
556
> 557 MODULE_DEVICE_TABLE(platform, twl6032_regulator_driver_ids);
558
559 static struct platform_driver twl6032_regulator_driver = {
560 .driver = {
---
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: 56830 bytes --]
^ permalink raw reply
* Re: [PATCH 4/5] regulator: Add support for TI TWL6032
From: Rosia, Nicolae @ 2016-11-26 20:24 UTC (permalink / raw)
To: lkp@intel.com
Cc: linux-kernel@vger.kernel.org, gg@slimlogic.co.uk,
robh+dt@kernel.org, devicetree@vger.kernel.org,
lee.jones@linaro.org, linux-omap@vger.kernel.org,
broonie@kernel.org, baruch@tkos.co.il, mark.rutland@arm.com,
lgirdwood@gmail.com, linux-arm-kernel@lists.infradead.org,
paul.gortmaker@windriver.com, tony@atomide.com
In-Reply-To: <201611270236.UFgW5zDA%fengguang.wu@intel.com>
Hi,
On Sun, 2016-11-27 at 02:55 +0800, kbuild test robot wrote:
> Hi Nicolae,
>
> [auto build test ERROR on omap/for-next]
> [also build test ERROR on v4.9-rc6]
> [cannot apply to next-20161125]
> [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/Nicolae-Rosia/mfd-tw
> l-improvements-and-new-regulator-driver/20161127-022201
> base: https://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-
> omap.git for-next
> config: i386-allmodconfig (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
>
> All error/warnings (new ones prefixed by >>):
>
> In file included from drivers/regulator/twl6032-regulator.c:11:0:
> > > drivers/regulator/twl6032-regulator.c:557:31: error:
> > > 'twl6032_regulator_driver_ids' undeclared here (not in a
> > > function)
>
> MODULE_DEVICE_TABLE(platform, twl6032_regulator_driver_ids);
> ^
> include/linux/module.h:213:21: note: in definition of macro
> 'MODULE_DEVICE_TABLE'
> extern const typeof(name)
> __mod_##type##__##name##_device_table \
> ^~~~
> > > include/linux/module.h:213:27: error:
> > > '__mod_platform__twl6032_regulator_driver_ids_device_table'
> > > aliased to undefined symbol 'twl6032_regulator_driver_ids'
>
> extern const typeof(name)
> __mod_##type##__##name##_device_table \
> ^
> > > drivers/regulator/twl6032-regulator.c:557:1: note: in expansion
> > > of macro 'MODULE_DEVICE_TABLE'
>
> MODULE_DEVICE_TABLE(platform, twl6032_regulator_driver_ids);
> ^~~~~~~~~~~~~~~~~~~
>
> vim +/twl6032_regulator_driver_ids +557 drivers/regulator/twl6032-
> regulator.c
>
> 551
> 552 static const struct of_device_id twl6032_dt_match[] = {
> 553 { .compatible = "ti,twl6032-regulator" },
> 554 { /* last entry */ }
> 555 };
> 556
> > 557 MODULE_DEVICE_TABLE(platform,
> twl6032_regulator_driver_ids);
> 558
> 559 static struct platform_driver twl6032_regulator_driver
> = {
> 560 .driver = {
Thanks, I did not notice this since I was only testing using built-in
module.
I will wait for comments before sending V2, untill then here's an
inline patch with the fix.
Best regards,
Nicolae
^ permalink raw reply
* Re: [PATCH] of: Fix issue where code would fall through to error case.
From: Frank Rowand @ 2016-11-26 21:39 UTC (permalink / raw)
To: Rob Herring, Moritz Fischer
Cc: Moritz Fischer,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Pantelis Antoniou, moritz-62aBmqa6xEOcmJEhUYGoYg,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_JsqJK=Jf7JCmqD8EgEg3ngONWz=1Fu-Jj5h6-wkskTk_iXw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 11/23/16 13:58, Rob Herring wrote:
> On Thu, Nov 17, 2016 at 6:10 PM, Moritz Fischer
> <moritz.fischer.private-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On Thu, Nov 17, 2016 at 4:02 PM, Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> On 11/17/16 15:40, Frank Rowand wrote:
>>>> On 11/17/16 15:25, Moritz Fischer wrote:
>>>>> No longer fall through into the error case that prints out
>>>>> an error if no error (err = 0) occurred.
>>>>>
>>>>> Fixes d9181b20a83(of: Add back an error message, restructured)
>>>>> Signed-off-by: Moritz Fischer <moritz.fischer-+aYTwkv1SeIAvxtiuMwx3w@public.gmane.org>
>>>>> ---
>>>>> drivers/of/resolver.c | 6 +++++-
>>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
>>>>> index 783bd09..785076d 100644
>>>>> --- a/drivers/of/resolver.c
>>>>> +++ b/drivers/of/resolver.c
>>>>> @@ -358,9 +358,13 @@ int of_resolve_phandles(struct device_node *overlay)
>>>>>
>>>>> err = update_usages_of_a_phandle_reference(overlay, prop, phandle);
>>>>> if (err)
>>>>> - break;
>>>>> + goto err_out;
>>>>> }
>>>>>
>>>>> + of_node_put(tree_symbols);
>>>>> +
>>>>> + return 0;
>>>>> +
>>>>> err_out:
>>>>> pr_err("overlay phandle fixup failed: %d\n", err);
>>>>> out:
>>>>
>>>> Thanks for catching that.
>>>>
>>>> Rob, please apply.
>>>>
>>>> Reviewed-by: Frank Rowand <frank.rowand-mEdOJwZ7QcZBDgjK7y7TUQ@public.gmane.org>
>>>>
>>>> -Frank
>>>
>>> On second thought, isn't the common pattern when clean up is needed for
>>> both the no-error path and the error path something like:
>>>
>>>
>>> out:
>>> of_node_put(tree_symbols);
>>> return err;
>>>
>>> err_out:
>>> pr_err("overlay phandle fixup failed: %d\n", err);
>>> goto out;
>>> }
>>>
>>>
>>> I don't have a strong opinion, whatever Rob wants to take is fine with me.
>>
>> Same here. I tried to avoid the jumping back part, but if that's the
>> common pattern,
>> I can submit a v2 doing that instead.
>
> Both are ugly. Just do:
>
> if (err)
> pr_err(...);
>
> Rob
Agreed. Thanks for the touch of sanity Rob.
-Frank
--
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 v4 1/2] i2c: aspeed: added driver for Aspeed I2C
From: Vladimir Zapolskiy @ 2016-11-26 22:31 UTC (permalink / raw)
To: Brendan Higgins, mark.rutland
Cc: wsa, robh+dt, linux-i2c, devicetree, joel, openbmc
In-Reply-To: <1478311099-6771-2-git-send-email-brendanhiggins@google.com>
Hello Brendan,
please find some review notes below.
On 11/05/2016 03:58 AM, Brendan Higgins wrote:
> Added initial master and slave support for Aspeed I2C controller.
> Supports fourteen busses present in ast24xx and ast25xx BMC SoCs by
> Aspeed.
>
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> ---
> Changes for v2:
> - Added single module_init (multiple was breaking some builds).
> Changes for v3:
> - Removed "bus" device tree param; now extracted from bus address offset
> Changes for v4:
> - I2C adapter number is now generated dynamically unless specified in alias.
> ---
> drivers/i2c/busses/Kconfig | 10 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-aspeed.c | 807 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 818 insertions(+)
> create mode 100644 drivers/i2c/busses/i2c-aspeed.c
>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index d252276..b6caa5d 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -1009,6 +1009,16 @@ config I2C_RCAR
> This driver can also be built as a module. If so, the module
> will be called i2c-rcar.
>
> +config I2C_ASPEED
> + tristate "Aspeed AST2xxx SoC I2C Controller"
> + depends on ARCH_ASPEED
> + help
> + If you say yes to this option, support will be included for the
> + Aspeed AST2xxx SoC I2C controller.
> +
> + This driver can also be built as a module. If so, the module
> + will be called i2c-aspeed.
> +
Please try to keep the list ordered, it would be better, if you
add I2C_ASPEED before I2C_AT91.
> comment "External I2C/SMBus adapter drivers"
>
> config I2C_DIOLAN_U2C
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 29764cc..826e780 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -99,6 +99,7 @@ obj-$(CONFIG_I2C_XILINX) += i2c-xiic.o
> obj-$(CONFIG_I2C_XLR) += i2c-xlr.o
> obj-$(CONFIG_I2C_XLP9XX) += i2c-xlp9xx.o
> obj-$(CONFIG_I2C_RCAR) += i2c-rcar.o
> +obj-$(CONFIG_I2C_ASPEED) += i2c-aspeed.o
>
Same comment as above, put it in front of i2c-at91.o
> # External I2C/SMBus adapter drivers
> obj-$(CONFIG_I2C_DIOLAN_U2C) += i2c-diolan-u2c.o
> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> new file mode 100644
> index 0000000..88e078a
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -0,0 +1,807 @@
> +/*
> + * I2C adapter for the ASPEED I2C bus.
> + *
> + * Copyright (C) 2012-2020 ASPEED Technology Inc.
I don't think that the copyright dated by 2020 in advance is legal here.
Please change it to the expected ...-2016 value.
> + * Copyright 2016 IBM Corporation
> + * Copyright 2016 Google, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/errno.h>
> +#include <linux/interrupt.h>
> +#include <linux/completion.h>
> +#include <linux/slab.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/err.h>
> +#include <linux/clk.h>
Please sort the headers alphabetically.
> +
> +/* I2C Register */
> +#define ASPEED_I2C_FUN_CTRL_REG 0x00
> +#define ASPEED_I2C_AC_TIMING_REG1 0x04
> +#define ASPEED_I2C_AC_TIMING_REG2 0x08
> +#define ASPEED_I2C_INTR_CTRL_REG 0x0c
> +#define ASPEED_I2C_INTR_STS_REG 0x10
> +#define ASPEED_I2C_CMD_REG 0x14
> +#define ASPEED_I2C_DEV_ADDR_REG 0x18
> +#define ASPEED_I2C_BYTE_BUF_REG 0x20
> +#define ASPEED_I2C_OFFSET_START 0x40
Unused macro, please remove.
> +#define ASPEED_I2C_OFFSET_INCREMENT 0x40
Unused macro, please remove.
> +
> +#define ASPEED_I2C_NUM_BUS 14
> +
> +/* Global Register Definition */
> +/* 0x00 : I2C Interrupt Status Register */
> +/* 0x08 : I2C Interrupt Target Assignment */
> +
> +/* Device Register Definition */
> +/* 0x00 : I2CD Function Control Register */
> +#define ASPEED_I2CD_MULTI_MASTER_DIS BIT(15)
Unused macro, please remove.
> +#define ASPEED_I2CD_SDA_DRIVE_1T_EN BIT(8)
Unused macro, please remove.
> +#define ASPEED_I2CD_M_SDA_DRIVE_1T_EN BIT(7)
> +#define ASPEED_I2CD_M_HIGH_SPEED_EN BIT(6)
> +#define ASPEED_I2CD_SLAVE_EN BIT(1)
Unused macro, please remove. You add slave support, may be you
need this control, but it is unused.
> +#define ASPEED_I2CD_MASTER_EN BIT(0)
> +
> +/* 0x08 : I2CD Clock and AC Timing Control Register #2 */
> +#define ASPEED_NO_TIMEOUT_CTRL 0
> +
> +
> +/* 0x0c : I2CD Interrupt Control Register &
> + * 0x10 : I2CD Interrupt Status Register
> + *
> + * These share bit definitions, so use the same values for the enable &
> + * status bits.
> + */
> +#define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT BIT(14)
> +#define ASPEED_I2CD_INTR_BUS_RECOVER_DONE BIT(13)
> +#define ASPEED_I2CD_INTR_SLAVE_MATCH BIT(7)
> +#define ASPEED_I2CD_INTR_SCL_TIMEOUT BIT(6)
> +#define ASPEED_I2CD_INTR_ABNORMAL BIT(5)
> +#define ASPEED_I2CD_INTR_NORMAL_STOP BIT(4)
> +#define ASPEED_I2CD_INTR_ARBIT_LOSS BIT(3)
> +#define ASPEED_I2CD_INTR_RX_DONE BIT(2)
> +#define ASPEED_I2CD_INTR_TX_NAK BIT(1)
> +#define ASPEED_I2CD_INTR_TX_ACK BIT(0)
> +
> +/* 0x14 : I2CD Command/Status Register */
> +#define ASPEED_I2CD_SCL_LINE_STS BIT(18)
> +#define ASPEED_I2CD_SDA_LINE_STS BIT(17)
> +#define ASPEED_I2CD_BUS_BUSY_STS BIT(16)
> +#define ASPEED_I2CD_BUS_RECOVER_CMD BIT(11)
> +
> +/* Command Bit */
> +#define ASPEED_I2CD_M_STOP_CMD BIT(5)
> +#define ASPEED_I2CD_M_S_RX_CMD_LAST BIT(4)
> +#define ASPEED_I2CD_M_RX_CMD BIT(3)
> +#define ASPEED_I2CD_S_TX_CMD BIT(2)
> +#define ASPEED_I2CD_M_TX_CMD BIT(1)
> +#define ASPEED_I2CD_M_START_CMD BIT(0)
> +
> +/* 0x18 : I2CD Slave Device Address Register */
> +#define ASPEED_I2CD_DEV_ADDR_MASK GENMASK(6, 0)
> +
> +enum aspeed_i2c_slave_state {
> + ASPEED_I2C_SLAVE_START,
> + ASPEED_I2C_SLAVE_READ_REQUESTED,
> + ASPEED_I2C_SLAVE_READ_PROCESSED,
> + ASPEED_I2C_SLAVE_WRITE_REQUESTED,
> + ASPEED_I2C_SLAVE_WRITE_RECEIVED,
> + ASPEED_I2C_SLAVE_STOP,
> +};
> +
> +struct aspeed_i2c_bus {
> + struct i2c_adapter adap;
> + struct device *dev;
> + void __iomem *base;
> + spinlock_t lock;
> + struct completion cmd_complete;
> + int irq;
> + /* Transaction state. */
> + struct i2c_msg *msg;
> + int msg_pos;
> + u32 cmd_err;
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> + struct i2c_client *slave;
> + enum aspeed_i2c_slave_state slave_state;
> +#endif
> +};
> +
> +struct aspeed_i2c_controller {
> + struct device *dev;
> + void __iomem *base;
> + int irq;
> + struct irq_domain *irq_domain;
> +};
> +
> +static inline void aspeed_i2c_write(struct aspeed_i2c_bus *bus, u32 val,
> + u32 reg)
> +{
> + writel(val, bus->base + reg);
> +}
> +
> +static inline u32 aspeed_i2c_read(struct aspeed_i2c_bus *bus, u32 reg)
> +{
> + return readl(bus->base + reg);
> +}
> +
> +static u8 aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
The function may return errors as negative numbers, please
change its return type to 'int'.
> +{
> + u32 command;
> + unsigned long time_left;
> + unsigned long flags;
> + int ret = 0;
Please whenever possible use "reverse christmas tree" order while
declaring local variables, this applies to all functions, here it
should be
unsigned long time_left, flags;
int ret = 0;
u32 command;
> +
> + spin_lock_irqsave(&bus->lock, flags);
> + command = aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG);
> + /* Bus is idle: no recovery needed. */
> + if ((command & ASPEED_I2CD_SDA_LINE_STS) &&
> + (command & ASPEED_I2CD_SCL_LINE_STS))
> + goto out;
> +
> + dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
> + command);
> +
> + /* Bus held: put bus in stop state. */
> + if ((command & ASPEED_I2CD_SDA_LINE_STS) &&
> + !(command & ASPEED_I2CD_SCL_LINE_STS)) {
> + aspeed_i2c_write(bus, ASPEED_I2CD_M_STOP_CMD,
> + ASPEED_I2C_CMD_REG);
> + reinit_completion(&bus->cmd_complete);
> + spin_unlock_irqrestore(&bus->lock, flags);
> +
> + time_left = wait_for_completion_interruptible_timeout(
> + &bus->cmd_complete, bus->adap.timeout * HZ);
> +
> + spin_lock_irqsave(&bus->lock, flags);
> + if (time_left == 0)
> + ret = -ETIMEDOUT;
> + else if (bus->cmd_err)
> + ret = -EIO;
Since you use wait_for_completion_interruptible_timeout(), please
handle possible -ERESTARTSYS error if the execution is interrupted.
You have 5 quite similar wait_for_completion_interruptible_timeout()
functions in the driver, please add it to all of them, or you may
think how to wrap it properly into a new function.
> + /* Bus error. */
> + } else if (!(command & ASPEED_I2CD_SDA_LINE_STS)) {
You don't need to introduce "else if" here, at this point
!(command & ASPEED_I2CD_SDA_LINE_STS) expression is always true.
Please drop dev_dbg("bus hang") statement from above (or put it to
an if-branch from two ones described below) and for simplicity
rearrange the code:
if (command & ASPEED_I2CD_SDA_LINE_STS) {
/* Bus is idle: no recovery needed. */
if (command & ASPEED_I2CD_SCL_LINE_STS)
goto out;
/* Bus held: put bus in stop state. */
.....
} else {
/* Bus error. */
.....
}
> + aspeed_i2c_write(bus, ASPEED_I2CD_BUS_RECOVER_CMD,
> + ASPEED_I2C_CMD_REG);
> + reinit_completion(&bus->cmd_complete);
> + spin_unlock_irqrestore(&bus->lock, flags);
> +
> + time_left = wait_for_completion_interruptible_timeout(
> + &bus->cmd_complete, bus->adap.timeout * HZ);
> +
> + spin_lock_irqsave(&bus->lock, flags);
> + if (time_left == 0)
> + ret = -ETIMEDOUT;
> + else if (bus->cmd_err)
> + ret = -EIO;
> + /* Recovery failed. */
> + else if (!(aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG) &
> + ASPEED_I2CD_SDA_LINE_STS))
> + ret = -EIO;
> + }
> +
> +out:
> + spin_unlock_irqrestore(&bus->lock, flags);
Please insert an empty line here to improve readability.
> + return ret;
> +}
> +
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> +static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus)
> +{
> + bool irq_handled = true;
> + u32 command;
> + u32 irq_status;
> + u32 status_ack = 0;
> + u8 value;
> + struct i2c_client *slave = bus->slave;
See a comment above about order of declared local variables.
> +
> + spin_lock(&bus->lock);
> + if (!slave) {
> + irq_handled = false;
> + goto out;
> + }
Add an empty line here.
> + command = aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG);
> + irq_status = aspeed_i2c_read(bus, ASPEED_I2C_INTR_STS_REG);
> +
> + /* Slave was requested, restart state machine. */
> + if (irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH) {
> + status_ack |= ASPEED_I2CD_INTR_SLAVE_MATCH;
> + bus->slave_state = ASPEED_I2C_SLAVE_START;
> + }
Add an empty line here.
> + /* Slave is not currently active, irq was for someone else. */
> + if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) {
> + irq_handled = false;
> + goto out;
> + }
> +
> + dev_dbg(bus->dev, "slave irq status 0x%08x, cmd 0x%08x\n",
> + irq_status, command);
> +
> + /* Slave was sent something. */
> + if (irq_status & ASPEED_I2CD_INTR_RX_DONE) {
> + value = aspeed_i2c_read(bus, ASPEED_I2C_BYTE_BUF_REG) >> 8;
> + /* Handle address frame. */
> + if (bus->slave_state == ASPEED_I2C_SLAVE_START) {
> + if (value & 0x1)
> + bus->slave_state =
> + ASPEED_I2C_SLAVE_READ_REQUESTED;
> + else
> + bus->slave_state =
> + ASPEED_I2C_SLAVE_WRITE_REQUESTED;
> + }
> + status_ack |= ASPEED_I2CD_INTR_RX_DONE;
> + }
> +
> + /* Slave was asked to stop. */
> + if (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) {
> + status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP;
> + bus->slave_state = ASPEED_I2C_SLAVE_STOP;
> + }
Add an empty line here.
> + if (irq_status & ASPEED_I2CD_INTR_TX_NAK) {
> + status_ack |= ASPEED_I2CD_INTR_TX_NAK;
> + bus->slave_state = ASPEED_I2C_SLAVE_STOP;
> + }
> +
> + if (bus->slave_state == ASPEED_I2C_SLAVE_READ_REQUESTED) {
> + if (irq_status & ASPEED_I2CD_INTR_TX_ACK)
> + dev_err(bus->dev, "Unexpected ACK on read request.\n");
> + bus->slave_state = ASPEED_I2C_SLAVE_READ_PROCESSED;
> +
> + i2c_slave_event(slave, I2C_SLAVE_READ_REQUESTED, &value);
> + aspeed_i2c_write(bus, value, ASPEED_I2C_BYTE_BUF_REG);
> + aspeed_i2c_write(bus, ASPEED_I2CD_S_TX_CMD, ASPEED_I2C_CMD_REG);
> + } else if (bus->slave_state == ASPEED_I2C_SLAVE_READ_PROCESSED) {
> + status_ack |= ASPEED_I2CD_INTR_TX_ACK;
> + if (!(irq_status & ASPEED_I2CD_INTR_TX_ACK))
> + dev_err(bus->dev,
> + "Expected ACK after processed read.\n");
> + i2c_slave_event(slave, I2C_SLAVE_READ_PROCESSED, &value);
> + aspeed_i2c_write(bus, value, ASPEED_I2C_BYTE_BUF_REG);
> + aspeed_i2c_write(bus, ASPEED_I2CD_S_TX_CMD, ASPEED_I2C_CMD_REG);
> + } else if (bus->slave_state == ASPEED_I2C_SLAVE_WRITE_REQUESTED) {
> + bus->slave_state = ASPEED_I2C_SLAVE_WRITE_RECEIVED;
> + i2c_slave_event(slave, I2C_SLAVE_WRITE_REQUESTED, &value);
> + } else if (bus->slave_state == ASPEED_I2C_SLAVE_WRITE_RECEIVED) {
> + i2c_slave_event(slave, I2C_SLAVE_WRITE_RECEIVED, &value);
> + } else if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) {
> + i2c_slave_event(slave, I2C_SLAVE_STOP, &value);
> + }
Please change this if-else-if-else-if-... construction of state machine
changes into switch-case.
> +
> + if (status_ack != irq_status)
> + dev_err(bus->dev,
> + "irq handled != irq. expected %x, but was %x\n",
> + irq_status, status_ack);
> + aspeed_i2c_write(bus, status_ack, ASPEED_I2C_INTR_STS_REG);
> +
> +out:
> + spin_unlock(&bus->lock);
> + return irq_handled;
> +}
> +#endif
> +
> +static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
> +{
> + const u32 errs = ASPEED_I2CD_INTR_ARBIT_LOSS |
> + ASPEED_I2CD_INTR_ABNORMAL |
> + ASPEED_I2CD_INTR_SCL_TIMEOUT |
> + ASPEED_I2CD_INTR_SDA_DL_TIMEOUT |
> + ASPEED_I2CD_INTR_TX_NAK;
You don't need this local variable, it is used only once, please
declare a macro and put it somewhere close to ASPEED_I2CD_INTR_*
macro in the beginning of the file.
> + u32 irq_status;
> +
> + spin_lock(&bus->lock);
> + irq_status = aspeed_i2c_read(bus, ASPEED_I2C_INTR_STS_REG);
> + bus->cmd_err = irq_status & errs;
> +
> + dev_dbg(bus->dev, "master irq status 0x%08x\n", irq_status);
> +
> + /* No message to transfer. */
> + if (bus->cmd_err ||
> + (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) ||
> + (irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE)) {
> + complete(&bus->cmd_complete);
> + goto out;
> + } else if (!bus->msg || bus->msg_pos >= bus->msg->len)
> + goto out;
> +
> + if ((bus->msg->flags & I2C_M_RD) &&
> + (irq_status & ASPEED_I2CD_INTR_RX_DONE)) {
> + bus->msg->buf[bus->msg_pos++] = aspeed_i2c_read(
> + bus, ASPEED_I2C_BYTE_BUF_REG) >> 8;
> + if (bus->msg_pos + 1 < bus->msg->len)
> + aspeed_i2c_write(bus, ASPEED_I2CD_M_RX_CMD,
> + ASPEED_I2C_CMD_REG);
> + else if (bus->msg_pos < bus->msg->len)
> + aspeed_i2c_write(bus, ASPEED_I2CD_M_RX_CMD |
> + ASPEED_I2CD_M_S_RX_CMD_LAST,
> + ASPEED_I2C_CMD_REG);
> + } else if (!(bus->msg->flags & I2C_M_RD) &&
> + (irq_status & ASPEED_I2CD_INTR_TX_ACK)) {
> + aspeed_i2c_write(bus, bus->msg->buf[bus->msg_pos++],
> + ASPEED_I2C_BYTE_BUF_REG);
> + aspeed_i2c_write(bus, ASPEED_I2CD_M_TX_CMD, ASPEED_I2C_CMD_REG);
> + }
> +
> + /* Transmission complete: notify caller. */
> + if (bus->msg_pos >= bus->msg->len)
> + complete(&bus->cmd_complete);
> +out:
> + aspeed_i2c_write(bus, irq_status, ASPEED_I2C_INTR_STS_REG);
> + spin_unlock(&bus->lock);
Please insert an empty line here to improve readability.
> + return true;
> +}
> +
> +static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
> +{
> + struct aspeed_i2c_bus *bus = dev_id;
> +
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> + if (aspeed_i2c_slave_irq(bus)) {
> + dev_dbg(bus->dev, "irq handled by slave.\n");
> + return IRQ_HANDLED;
> + }
> +#endif
Please insert an empty line here to improve readability.
> + if (aspeed_i2c_master_irq(bus)) {
> + dev_dbg(bus->dev, "irq handled by master.\n");
> + return IRQ_HANDLED;
> + }
Please insert an empty line here to improve readability.
> + dev_err(bus->dev, "irq not handled properly!\n");
> + return IRQ_HANDLED;
If interrupt is not handled, return IRQ_NONE.
> +}
> +
> +static int aspeed_i2c_master_single_xfer(struct i2c_adapter *adap,
> + struct i2c_msg *msg)
> +{
> + struct aspeed_i2c_bus *bus = adap->algo_data;
> + unsigned long flags;
> + u8 slave_addr;
> + u32 command = ASPEED_I2CD_M_START_CMD | ASPEED_I2CD_M_TX_CMD;
> + int ret = msg->len;
> + unsigned long time_left;
> +
> + spin_lock_irqsave(&bus->lock, flags);
> + bus->msg = msg;
> + bus->msg_pos = 0;
> + slave_addr = msg->addr << 1;
Please insert an empty line here to improve readability.
> + if (msg->flags & I2C_M_RD) {
> + slave_addr |= 1;
> + command |= ASPEED_I2CD_M_RX_CMD;
> + if (msg->len == 1)
> + command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
> + }
Please insert an empty line here to improve readability.
> + aspeed_i2c_write(bus, slave_addr, ASPEED_I2C_BYTE_BUF_REG);
> + aspeed_i2c_write(bus, command, ASPEED_I2C_CMD_REG);
> + reinit_completion(&bus->cmd_complete);
> + spin_unlock_irqrestore(&bus->lock, flags);
> +
> + time_left = wait_for_completion_interruptible_timeout(
> + &bus->cmd_complete, bus->adap.timeout * HZ * msg->len);
> + if (time_left == 0)
> + return -ETIMEDOUT;
> +
> + spin_lock_irqsave(&bus->lock, flags);
> + if (bus->cmd_err)
> + ret = -EIO;
Please insert an empty line here to improve readability.
> + bus->msg = NULL;
> + spin_unlock_irqrestore(&bus->lock, flags);
> +
> + return ret;
> +}
> +
> +static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
> + struct i2c_msg *msgs, int num)
> +{
> + struct aspeed_i2c_bus *bus = adap->algo_data;
> + int ret;
> + int i;
> + unsigned long flags;
> + unsigned long time_left;
> +
> + /* If bus is busy, attempt recovery. We assume a single master
> + * environment.
> + */
> + if (aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG) &
> + ASPEED_I2CD_BUS_BUSY_STS) {
> + ret = aspeed_i2c_recover_bus(bus);
> + if (ret)
> + return ret;
> + }
> +
> + for (i = 0; i < num; i++) {
> + ret = aspeed_i2c_master_single_xfer(adap, &msgs[i]);
> + if (ret < 0)
> + break;
> + /* TODO: Support other forms of I2C protocol mangling. */
> + if (msgs[i].flags & I2C_M_STOP) {
> + spin_lock_irqsave(&bus->lock, flags);
> + aspeed_i2c_write(bus, ASPEED_I2CD_M_STOP_CMD,
> + ASPEED_I2C_CMD_REG);
> + reinit_completion(&bus->cmd_complete);
> + spin_unlock_irqrestore(&bus->lock, flags);
> +
> + time_left = wait_for_completion_interruptible_timeout(
> + &bus->cmd_complete,
> + bus->adap.timeout * HZ);
> + if (time_left == 0)
> + return -ETIMEDOUT;
> + }
> + }
> +
> + spin_lock_irqsave(&bus->lock, flags);
> + aspeed_i2c_write(bus, ASPEED_I2CD_M_STOP_CMD, ASPEED_I2C_CMD_REG);
> + reinit_completion(&bus->cmd_complete);
> + spin_unlock_irqrestore(&bus->lock, flags);
> +
> + time_left = wait_for_completion_interruptible_timeout(
> + &bus->cmd_complete, bus->adap.timeout * HZ);
> + if (time_left == 0)
> + return -ETIMEDOUT;
> +
> + /* If nothing went wrong, return number of messages transferred. */
> + if (ret < 0)
> + return ret;
> + else
> + return i;
> +}
> +
> +static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
> +{
> + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
> +}
> +
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> +static int aspeed_i2c_reg_slave(struct i2c_client *client)
> +{
> + struct aspeed_i2c_bus *bus;
> + unsigned long flags;
> + u32 addr_reg_val;
> + u32 func_ctrl_reg_val;
> +
> + bus = client->adapter->algo_data;
> + spin_lock_irqsave(&bus->lock, flags);
> + if (bus->slave) {
> + spin_unlock_irqrestore(&bus->lock, flags);
> + return -EINVAL;
> + }
> +
> + /* Set slave addr. */
> + addr_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_DEV_ADDR_REG);
> + addr_reg_val &= ~ASPEED_I2CD_DEV_ADDR_MASK;
> + addr_reg_val |= client->addr & ASPEED_I2CD_DEV_ADDR_MASK;
> + aspeed_i2c_write(bus, addr_reg_val, ASPEED_I2C_DEV_ADDR_REG);
> +
> + /* Switch from master mode to slave mode. */
> + func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
> + func_ctrl_reg_val &= ~ASPEED_I2CD_MASTER_EN;
> + func_ctrl_reg_val |= ASPEED_I2CD_SLAVE_EN;
> + aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
> +
> + bus->slave = client;
> + bus->slave_state = ASPEED_I2C_SLAVE_STOP;
> + spin_unlock_irqrestore(&bus->lock, flags);
Please insert an empty line here to improve readability.
> + return 0;
> +}
> +
> +static int aspeed_i2c_unreg_slave(struct i2c_client *client)
> +{
> + struct aspeed_i2c_bus *bus = client->adapter->algo_data;
> + unsigned long flags;
> + u32 func_ctrl_reg_val;
> +
> + spin_lock_irqsave(&bus->lock, flags);
> + if (!bus->slave) {
> + spin_unlock_irqrestore(&bus->lock, flags);
> + return -EINVAL;
> + }
> +
> + /* Switch from slave mode to master mode. */
> + func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
> + func_ctrl_reg_val &= ~ASPEED_I2CD_SLAVE_EN;
> + func_ctrl_reg_val |= ASPEED_I2CD_MASTER_EN;
> + aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
> +
> + bus->slave = NULL;
> + spin_unlock_irqrestore(&bus->lock, flags);
Please insert an empty line here to improve readability.
> + return 0;
> +}
> +#endif
> +
> +static const struct i2c_algorithm aspeed_i2c_algo = {
> + .master_xfer = aspeed_i2c_master_xfer,
> + .functionality = aspeed_i2c_functionality,
> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> + .reg_slave = aspeed_i2c_reg_slave,
> + .unreg_slave = aspeed_i2c_unreg_slave,
> +#endif
> +};
> +
> +static u32 aspeed_i2c_get_clk_reg_val(u32 divider_ratio)
> +{
> + unsigned int inc = 0, div;
> + u32 scl_low, scl_high, data;
> +
> + for (div = 0; divider_ratio >= 16; div++) {
> + inc |= (divider_ratio & 1);
> + divider_ratio >>= 1;
> + }
Please insert an empty line here to improve readability.
> + divider_ratio += inc;
> + scl_low = (divider_ratio >> 1) - 1;
> + scl_high = divider_ratio - scl_low - 2;
> + data = 0x77700300 | (scl_high << 16) | (scl_low << 12) | div;
> + return data;
> +}
> +
> +static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus,
> + struct platform_device *pdev)
> +{
> + struct clk *pclk;
> + u32 clk_freq;
> + u32 divider_ratio;
> + int ret;
> +
> + pclk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(pclk)) {
> + dev_err(&pdev->dev, "clk_get failed\n");
> + return PTR_ERR(pclk);
> + }
> + ret = of_property_read_u32(pdev->dev.of_node,
> + "clock-frequency", &clk_freq);
> + if (ret < 0) {
> + dev_err(&pdev->dev,
> + "Could not read clock-frequency property\n");
> + clk_freq = 100000;
> + }
> + divider_ratio = clk_get_rate(pclk) / clk_freq;
> + /* We just need the clock rate, we don't actually use the clk object. */
> + devm_clk_put(&pdev->dev, pclk);
Does the controller have a clock supply? If yes, shall the clock be
enabled before issuing command to the controller?
> +
> + /* Set AC Timing */
> + if (clk_freq / 1000 > 400) {
> + aspeed_i2c_write(bus, aspeed_i2c_read(bus,
> + ASPEED_I2C_FUN_CTRL_REG) |
> + ASPEED_I2CD_M_HIGH_SPEED_EN |
> + ASPEED_I2CD_M_SDA_DRIVE_1T_EN |
> + ASPEED_I2CD_SDA_DRIVE_1T_EN,
> + ASPEED_I2C_FUN_CTRL_REG);
> +
> + aspeed_i2c_write(bus, 0x3, ASPEED_I2C_AC_TIMING_REG2);
> + aspeed_i2c_write(bus, aspeed_i2c_get_clk_reg_val(divider_ratio),
> + ASPEED_I2C_AC_TIMING_REG1);
> + } else {
> + aspeed_i2c_write(bus, aspeed_i2c_get_clk_reg_val(divider_ratio),
> + ASPEED_I2C_AC_TIMING_REG1);
> + aspeed_i2c_write(bus, ASPEED_NO_TIMEOUT_CTRL,
> + ASPEED_I2C_AC_TIMING_REG2);
> + }
> +
> + return 0;
> +}
> +
> +static void noop(struct irq_data *data) { }
> +
> +static struct irq_chip aspeed_i2c_irqchip = {
> + .name = "ast-i2c",
> + .irq_unmask = noop,
> + .irq_mask = noop,
Thomas or Marc should review the irqchip code for correctness,
inform them.
> +};
> +
> +static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> +{
> + struct aspeed_i2c_bus *bus;
> + struct aspeed_i2c_controller *controller =
> + dev_get_drvdata(pdev->dev.parent);
How do you ensure that "controller" device _driver_ is initialized
at this point? This is a critical race condition.
> + struct resource *res;
> + int ret, irq;
> + u32 hwirq;
> +
> + bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> + if (!bus)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + bus->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(bus->base))
> + return PTR_ERR(bus->base);
> +
> + bus->irq = platform_get_irq(pdev, 0);
> + if (bus->irq < 0)
> + return -ENXIO;
Remove check for error here, on error it will be returned by the
following devm_request_irq().
> + ret = of_property_read_u32(pdev->dev.of_node, "interrupts", &hwirq);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "no I2C 'interrupts' property\n");
> + return -ENXIO;
return -EINVAL;
> + }
> + irq = irq_create_mapping(controller->irq_domain, hwirq);
> + irq_set_chip_data(irq, controller);
> + irq_set_chip_and_handler(irq, &aspeed_i2c_irqchip, handle_simple_irq);
> + ret = devm_request_irq(&pdev->dev, bus->irq, aspeed_i2c_bus_irq,
> + 0, dev_name(&pdev->dev), bus);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to request interrupt\n");
> + return -ENXIO;
return ret;
In general I'm very surprised to see irqchip declaration and
initialization as a part of an I2C bus driver.
What is it used for? If you keep it, most probably IRQCHIP maintainers
(Thomas, Marc) shall review and ack the code.
> + }
> +
> + /* Initialize the I2C adapter */
> + spin_lock_init(&bus->lock);
> + init_completion(&bus->cmd_complete);
> + bus->adap.owner = THIS_MODULE;
> + bus->adap.retries = 0;
> + bus->adap.timeout = 5;
I don't think that this is a correct value, because here timeout unit
is in jiffies. Please update it here and everywhere in the driver,
where you use the value.
> + bus->adap.algo = &aspeed_i2c_algo;
> + bus->adap.algo_data = bus;
> + bus->adap.dev.parent = &pdev->dev;
> + bus->adap.dev.of_node = pdev->dev.of_node;
> + snprintf(bus->adap.name, sizeof(bus->adap.name), "Aspeed i2c");
> +
> + bus->dev = &pdev->dev;
> +
> + /* reset device: disable master & slave functions */
> + aspeed_i2c_write(bus, 0, ASPEED_I2C_FUN_CTRL_REG);
> +
> + ret = aspeed_i2c_init_clk(bus, pdev);
> + if (ret < 0)
> + return ret;
I'm not sure here, probably you start leaking resources allocated by
irq_create_mapping().
> +
> + /* Enable Master Mode */
> + aspeed_i2c_write(bus, aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG) |
> + ASPEED_I2CD_MASTER_EN |
> + ASPEED_I2CD_MULTI_MASTER_DIS, ASPEED_I2C_FUN_CTRL_REG);
> +
> + /* Set interrupt generation of I2C controller */
> + aspeed_i2c_write(bus, ASPEED_I2CD_INTR_SDA_DL_TIMEOUT |
> + ASPEED_I2CD_INTR_BUS_RECOVER_DONE |
> + ASPEED_I2CD_INTR_SCL_TIMEOUT |
> + ASPEED_I2CD_INTR_ABNORMAL |
> + ASPEED_I2CD_INTR_NORMAL_STOP |
> + ASPEED_I2CD_INTR_ARBIT_LOSS |
> + ASPEED_I2CD_INTR_RX_DONE |
> + ASPEED_I2CD_INTR_TX_NAK |
> + ASPEED_I2CD_INTR_TX_ACK,
Please declare a macro which combines all enabled interrupt events, and
put it somewhere close to ASPEED_I2CD_INTR_* macro definitions.
> + ASPEED_I2C_INTR_CTRL_REG);
> +
> + ret = i2c_add_adapter(&bus->adap);
> + if (ret < 0)
> + return -ENXIO;
Incorrect, you should propogate the error and "return ret;" here.
> +
> + platform_set_drvdata(pdev, bus);
> +
> + dev_info(bus->dev, "i2c bus %d registered, irq %d\n",
> + bus->adap.nr, bus->irq);
> +
> + return 0;
> +}
> +
> +static int aspeed_i2c_remove_bus(struct platform_device *pdev)
> +{
> + struct aspeed_i2c_bus *bus = platform_get_drvdata(pdev);
> +
> + i2c_del_adapter(&bus->adap);
Please insert an empty line here to improve readability.
> + return 0;
> +}
> +
> +static const struct of_device_id aspeed_i2c_bus_of_table[] = {
> + { .compatible = "aspeed,ast2400-i2c-bus", },
> + { .compatible = "aspeed,ast2500-i2c-bus", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, aspeed_i2c_bus_of_table);
> +
> +static struct platform_driver aspeed_i2c_bus_driver = {
> + .probe = aspeed_i2c_probe_bus,
> + .remove = aspeed_i2c_remove_bus,
> + .driver = {
> + .name = "ast-i2c-bus",
> + .of_match_table = aspeed_i2c_bus_of_table,
> + },
> +};
> +
> +static void aspeed_i2c_controller_irq(struct irq_desc *desc)
> +{
> + struct aspeed_i2c_controller *c = irq_desc_get_handler_data(desc);
> + unsigned long p, status;
> + unsigned int bus_irq;
> +
> + status = readl(c->base);
> + for_each_set_bit(p, &status, ASPEED_I2C_NUM_BUS) {
> + bus_irq = irq_find_mapping(c->irq_domain, p);
> + generic_handle_irq(bus_irq);
> + }
This is a processing of a cascaded interrupt, you should wrap
the code into chained_irq_enter()/chained_irq_exit().
> +}
> +
> +static int aspeed_i2c_probe_controller(struct platform_device *pdev)
> +{
> + struct aspeed_i2c_controller *controller;
> + struct device_node *np;
> + struct resource *res;
> +
> + controller = kzalloc(sizeof(*controller), GFP_KERNEL);
> + if (!controller)
> + return -ENOMEM;
You don't free the allocated memory on driver release, please convert
allocation to devm_kzalloc().
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + controller->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(controller->base))
> + return PTR_ERR(controller->base);
> +
> + controller->irq = platform_get_irq(pdev, 0);
> + if (controller->irq < 0)
> + return -ENXIO;
> +
> + controller->irq_domain = irq_domain_add_linear(pdev->dev.of_node,
> + ASPEED_I2C_NUM_BUS, &irq_domain_simple_ops, NULL);
> + if (!controller->irq_domain)
> + return -ENXIO;
return -ENOMEM is the correct error here.
Please insert an empty line here to improve readability.
> + controller->irq_domain->name = "ast-i2c-domain";
> +
> + irq_set_chained_handler_and_data(controller->irq,
> + aspeed_i2c_controller_irq, controller);
> +
> + controller->dev = &pdev->dev;
> +
> + platform_set_drvdata(pdev, controller);
> +
> + dev_info(controller->dev, "i2c controller registered, irq %d\n",
> + controller->irq);
> +
> + for_each_child_of_node(pdev->dev.of_node, np) {
> + of_platform_device_create(np, NULL, &pdev->dev);
> + of_node_put(np);
This is invalid use of of_node_put(np) inside for_each_child_of_node().
Just remove it to make the statement valid.
> + }
> +
> + return 0;
> +}
> +
> +static int aspeed_i2c_remove_controller(struct platform_device *pdev)
> +{
> + struct aspeed_i2c_controller *controller = platform_get_drvdata(pdev);
> +
> + irq_domain_remove(controller->irq_domain);
Please insert an empty line here to improve readability.
> + return 0;
> +}
> +
> +static const struct of_device_id aspeed_i2c_controller_of_table[] = {
> + { .compatible = "aspeed,ast2400-i2c-controller", },
> + { .compatible = "aspeed,ast2500-i2c-controller", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, aspeed_i2c_controller_of_table);
> +
> +static struct platform_driver aspeed_i2c_controller_driver = {
> + .probe = aspeed_i2c_probe_controller,
> + .remove = aspeed_i2c_remove_controller,
> + .driver = {
> + .name = "ast-i2c-controller",
> + .of_match_table = aspeed_i2c_controller_of_table,
> + },
> +};
> +
> +static int __init aspeed_i2c_driver_init(void)
> +{
> + int ret;
> +
> + ret = platform_driver_register(&aspeed_i2c_controller_driver);
> + if (ret < 0)
> + return ret;
Please insert an empty line here to improve readability.
> + return platform_driver_register(&aspeed_i2c_bus_driver);
No, on error you must unregister &aspeed_i2c_controller_driver.
> +}
> +module_init(aspeed_i2c_driver_init);
> +
> +static void __exit aspeed_i2c_driver_exit(void)
> +{
> + platform_driver_unregister(&aspeed_i2c_bus_driver);
> + platform_driver_unregister(&aspeed_i2c_controller_driver);
> +}
> +module_exit(aspeed_i2c_driver_exit);
> +
> +MODULE_AUTHOR("Brendan Higgins <brendanhiggins@google.com>");
> +MODULE_DESCRIPTION("Aspeed I2C Bus Driver");
> +MODULE_LICENSE("GPL");
>
^ permalink raw reply
* Re: [PATCH 1/2] Documentation: devicetree: clarify usage of the RGMII phy-modes
From: Florian Fainelli @ 2016-11-27 5:53 UTC (permalink / raw)
To: Martin Blumenstingl, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
sean.wang-NuS5LvNUpcJWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
jbrunet-rdvid1DuHRBWk0Htik3J/w
In-Reply-To: <20161125131201.19994-2-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
On 11/25/2016 05:12 AM, Martin Blumenstingl wrote:
> RGMII requires special RX and/or TX delays depending on the actual
> hardware circuit/wiring. These delays can be added by the MAC, the PHY
> or the designer of the circuit (the latter means that no delay has to
> be added by PHY or MAC).
> There are 4 RGMII phy-modes used describe where a delay should be
> applied:
> - rgmii: the RX and TX delays are either added by the MAC (where the
> exact delay is typically configurable, and can be turned off when no
> extra delay is needed) or not needed at all (because the hardware
> wiring adds the delay already). The PHY should neither add the RX nor
> TX delay in this case.
> - rgmii-rxid: configures the PHY to enable the RX delay. The MAC should
> not add the RX delay in this case.
> - rgmii-txid: configures the PHY to enable the TX delay. The MAC should
> not add the TX delay in this case.
> - rgmii-id: combines rgmii-rxid and rgmii-txid and thus configures the
> PHY to enable the RX and TX delays. The MAC should neither add the RX
> nor TX delay in this case.
>
> Document these cases in the ethernet.txt documentation to make it clear
> when to use each mode.
> If applied incorrectly one might end up with MAC and PHY both enabling
> for example the TX delay, which breaks ethernet TX traffic on 1000Mbit/s
> links.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Reviewed-by: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
--
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 2/2] net: phy: realtek: fix enabling of the TX-delay for RTL8211F
From: Florian Fainelli @ 2016-11-27 5:55 UTC (permalink / raw)
To: Martin Blumenstingl, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
sean.wang-NuS5LvNUpcJWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
jbrunet-rdvid1DuHRBWk0Htik3J/w
In-Reply-To: <20161125131201.19994-3-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
On 11/25/2016 05:12 AM, Martin Blumenstingl wrote:
> The old logic always enabled the TX-delay when the phy-mode was set to
> PHY_INTERFACE_MODE_RGMII. There are dedicated phy-modes which tell the
> PHY driver to enable the RX and/or TX delays:
> - PHY_INTERFACE_MODE_RGMII should disable the RX and TX delay in the
> PHY (if required, the MAC should add the delays in this case)
> - PHY_INTERFACE_MODE_RGMII_ID should enable RX and TX delay in the PHY
> - PHY_INTERFACE_MODE_RGMII_TXID should enable the TX delay in the PHY
> - PHY_INTERFACE_MODE_RGMII_RXID should enable the RX delay in the PHY
> (currently not supported by RTL8211F)
>
> With this patch we enable the TX delay for PHY_INTERFACE_MODE_RGMII_ID
> and PHY_INTERFACE_MODE_RGMII_TXID.
> Additionally we now explicity disable the TX-delay, which seems to be
> enabled automatically after a hard-reset of the PHY (by triggering it's
> reset pin) to get a consistent state (as defined by the phy-mode).
>
> This fixes a compatibility problem with some SoCs where the TX-delay was
> also added by the MAC. With the TX-delay being applied twice the TX
> clock was off and TX traffic was broken or very slow (<10Mbit/s) on
> 1000Mbit/s links.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Reviewed-by: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
--
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 1/2] Documentation: sample averaging for imx6ul_tsc
From: Guy Shapiro @ 2016-11-27 7:44 UTC (permalink / raw)
To: dmitry.torokhov
Cc: fabio.estevam, mark.rutland, Guy Shapiro, devicetree, haibo.chen,
robh+dt, linux-input, linux-arm-kernel
The i.MX6UL internal touchscreen controller contains an option to
average upon samples. This feature reduces noise from the produced
touch locations.
This patch introduces a new device tree optional property for this
feature. It provides control over the amount of averaged samples per
touch event.
The property was inspired by a similar property on the
"brcm,iproc-touchscreen" binding.
Signed-off-by: Guy Shapiro <guy.shapiro@mobi-wize.com>
---
.../devicetree/bindings/input/touchscreen/imx6ul_tsc.txt | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
index 853dff9..a66069f 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/imx6ul_tsc.txt
@@ -17,6 +17,13 @@ Optional properties:
This value depends on the touch screen.
- pre-charge-time: the touch screen need some time to precharge.
This value depends on the touch screen.
+- average-samples: Number of data samples which are averaged for each read.
+ Valid values 0-4
+ 0 = 1 sample
+ 1 = 4 samples
+ 2 = 8 samples
+ 3 = 16 samples
+ 4 = 32 samples
Example:
tsc: tsc@02040000 {
@@ -32,5 +39,6 @@ Example:
xnur-gpio = <&gpio1 3 GPIO_ACTIVE_LOW>;
measure-delay-time = <0xfff>;
pre-charge-time = <0xffff>;
+ average-samples = <4>;
status = "okay";
};
--
2.1.4
^ permalink raw reply related
* [PATCH 2/2] input: touchscreen: sample averaging for imx6ul_tsc
From: Guy Shapiro @ 2016-11-27 7:44 UTC (permalink / raw)
To: dmitry.torokhov
Cc: fabio.estevam, mark.rutland, Guy Shapiro, devicetree, haibo.chen,
robh+dt, linux-input, linux-arm-kernel
In-Reply-To: <1480232698-23075-1-git-send-email-guy.shapiro@mobi-wize.com>
The i.MX6UL internal touchscreen controller contains an option to
average upon samples. This feature reduces noise from the produced
touch locations.
This patch adds sample averaging support to the imx6ul_tsc device
driver.
Signed-off-by: Guy Shapiro <guy.shapiro@mobi-wize.com>
---
drivers/input/touchscreen/imx6ul_tsc.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/input/touchscreen/imx6ul_tsc.c b/drivers/input/touchscreen/imx6ul_tsc.c
index 8275267..31724d9 100644
--- a/drivers/input/touchscreen/imx6ul_tsc.c
+++ b/drivers/input/touchscreen/imx6ul_tsc.c
@@ -25,6 +25,7 @@
/* ADC configuration registers field define */
#define ADC_AIEN (0x1 << 7)
#define ADC_CONV_DISABLE 0x1F
+#define ADC_AVGE (0x1 << 5)
#define ADC_CAL (0x1 << 7)
#define ADC_CALF 0x2
#define ADC_12BIT_MODE (0x2 << 2)
@@ -32,6 +33,7 @@
#define ADC_CLK_DIV_8 (0x03 << 5)
#define ADC_SHORT_SAMPLE_MODE (0x0 << 4)
#define ADC_HARDWARE_TRIGGER (0x1 << 13)
+#define ADC_AVGS_SHIFT 14
#define SELECT_CHANNEL_4 0x04
#define SELECT_CHANNEL_1 0x01
#define DISABLE_CONVERSION_INT (0x0 << 7)
@@ -86,6 +88,7 @@ struct imx6ul_tsc {
int measure_delay_time;
int pre_charge_time;
+ int average_samples;
struct completion completion;
};
@@ -107,6 +110,8 @@ static int imx6ul_adc_init(struct imx6ul_tsc *tsc)
adc_cfg = readl(tsc->adc_regs + REG_ADC_CFG);
adc_cfg |= ADC_12BIT_MODE | ADC_IPG_CLK;
adc_cfg |= ADC_CLK_DIV_8 | ADC_SHORT_SAMPLE_MODE;
+ if (tsc->average_samples)
+ adc_cfg |= (tsc->average_samples - 1) << ADC_AVGS_SHIFT;
adc_cfg &= ~ADC_HARDWARE_TRIGGER;
writel(adc_cfg, tsc->adc_regs + REG_ADC_CFG);
@@ -118,6 +123,8 @@ static int imx6ul_adc_init(struct imx6ul_tsc *tsc)
/* start ADC calibration */
adc_gc = readl(tsc->adc_regs + REG_ADC_GC);
adc_gc |= ADC_CAL;
+ if (tsc->average_samples)
+ adc_gc |= ADC_AVGE;
writel(adc_gc, tsc->adc_regs + REG_ADC_GC);
timeout = wait_for_completion_timeout
@@ -450,6 +457,16 @@ static int imx6ul_tsc_probe(struct platform_device *pdev)
if (err)
tsc->pre_charge_time = 0xfff;
+ err = of_property_read_u32(np, "average-samples",
+ &tsc->average_samples);
+ if (err)
+ tsc->average_samples = 0;
+ if (tsc->average_samples > 4) {
+ dev_err(&pdev->dev, "average-samples (%u) must be [0-4]\n",
+ tsc->average_samples);
+ return -EINVAL;
+ }
+
err = input_register_device(tsc->input);
if (err) {
dev_err(&pdev->dev,
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v2 2/3] ARM: dts: sunxi: add support for Orange Pi Zero board
From: Icenowy Zheng @ 2016-11-27 9:36 UTC (permalink / raw)
To: Jonathan Corbet, Maxime Ripard, Chen-Yu Tsai, Mark Rutland,
Russell King, Hans de Goede
Cc: devicetree@vger.kernel.org, Vishnu Patekar, Arnd Bergmann,
linux-doc@vger.kernel.org, Andre Przywara,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20161121162421.800-2-icenowy@aosc.xyz>
22.11.2016, 00:26, "Icenowy Zheng" <icenowy@aosc.xyz>:
> Orange Pi Zero is a board that came with the new Allwinner H2+ SoC.
>
> Add a device tree file for it.
>
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> ---
> Changes since v2:
> - Use generic pinconf binding instead of legacy allwinner pinctrl binding.
> - removed uart3, which is not accessible on Orange Pi Zero.
> - Removed sun8i-h2plus.dtsi and make Orange Pi Zero dts directly include
> sun8i-h3.dtsi.
> - Removed allwinner,sun8i-h3 compatible.
>
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts | 137 +++++++++++++++++++++++
> 2 files changed, 138 insertions(+)
> create mode 100644 arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 802a10d..51a1dd7 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -834,6 +834,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
> sun8i-a33-sinlinx-sina33.dtb \
> sun8i-a83t-allwinner-h8homlet-v2.dtb \
> sun8i-a83t-cubietruck-plus.dtb \
> + sun8i-h2plus-orangepi-zero.dtb \
> sun8i-h3-bananapi-m2-plus.dtb \
> sun8i-h3-nanopi-neo.dtb \
> sun8i-h3-orangepi-2.dtb \
> diff --git a/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts b/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
> new file mode 100644
> index 0000000..b428e47
> --- /dev/null
> +++ b/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
> @@ -0,0 +1,137 @@
> +/*
> + * Copyright (C) 2016 Icenowy Zheng <icenowy@aosc.xyz>
> + *
> + * Based on sun8i-h3-orangepi-one.dts, which is:
> + * Copyright (C) 2016 Hans de Goede <hdegoede@redhat.com>
> + *
> + * 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 "sun8i-h3.dtsi"
> +#include "sunxi-common-regulators.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/pinctrl/sun4i-a10.h>
> +
> +/ {
> + model = "Xunlong Orange Pi Zero";
> + compatible = "xunlong,orangepi-zero", "allwinner,sun8i-h2plus";
> +
> + aliases {
> + serial0 = &uart0;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + leds {
> + compatible = "gpio-leds";
> + pinctrl-names = "default";
> + pinctrl-0 = <&leds_opi0>, <&leds_r_opi0>;
> +
> + pwr_led {
> + label = "orangepi:green:pwr";
> + gpios = <&r_pio 0 10 GPIO_ACTIVE_HIGH>;
> + default-state = "on";
> + };
> +
> + status_led {
> + label = "orangepi:red:status";
> + gpios = <&pio 0 17 GPIO_ACTIVE_HIGH>;
> + };
> + };
> +};
> +
> +&ehci1 {
> + status = "okay";
> +};
> +
> +&mmc0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin>;
> + vmmc-supply = <®_vcc3v3>;
> + bus-width = <4>;
> + cd-gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>; /* PF6 */
> + cd-inverted;
> + status = "okay";
> +};
> +
> +&ohci1 {
> + status = "okay";
> +};
> +
> +&pio {
> + leds_opi0: led_pins@0 {
> + pins = "PA17";
> + function = "gpio_out";
> + };
> +};
> +
> +&r_pio {
> + leds_r_opi0: led_pins@0 {
> + pins = "PL10";
> + function = "gpio_out";
> + };
> +};
> +
> +&uart0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart0_pins_a>;
> + status = "okay";
> +};
> +
> +&uart1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart1_pins>;
> + status = "disabled";
> +};
> +
> +&uart2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart2_pins>;
> + status = "disabled";
> +};
> +
> +&usbphy {
> + /* USB VBUS is always on */
> + status = "okay";
> +};
Something more interesting happened.
Xunlong made a add-on board for Orange Pi Zero, which exposes the two USB Controllers exported at expansion bus as USB Type-A connectors.
Also it exposes a analog A/V jack and a microphone.
Should I enable {e,o}hci{2.3} in the device tree?
> --
> 2.10.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 3/8] drivers:input:tsc2007: add iio interface to read external ADC input and temperature
From: Jonathan Cameron @ 2016-11-27 11:02 UTC (permalink / raw)
To: H. Nikolaus Schaller, Jonathan Cameron
Cc: Sebastian Reichel, Dmitry Torokhov, Mark Rutland,
Benoît Cousson, Tony Lindgren, Russell King, Arnd Bergmann,
Michael Welling, Mika Penttilä, Javier Martinez Canillas,
Igor Grinberg, Andrew F. Davis, Mark Brown, Rob Herring,
Alexander Stein, Eric Engestrom, Hans de Goede,
Benjamin Tissoires
In-Reply-To: <811B6F6A-2E3D-45B4-A984-74ABE0E37192-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
On 24/11/16 18:05, H. Nikolaus Schaller wrote:
>
>> Am 24.11.2016 um 18:38 schrieb Jonathan Cameron <jic23-tko9wxEg+fIOOJlXag/Snyp2UmYkHbXO@public.gmane.org>:
>>
>>
>>
>> On 22 November 2016 14:02:30 GMT+00:00, "H. Nikolaus Schaller" <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org> wrote:
>>> The tsc2007 chip not only has a resistive touch screen controller but
>>> also an external AUX adc imput which can be used for an ambient
>>> light sensor, battery voltage monitoring or any general purpose.
>>>
>>> Additionally it can measure the chip temperature.
>>>
>>> This extension provides an iio interface for these adc channels.
>>>
>>> Since it is not wasting much resources and is very straightforward,
>>> we simply provide all other adc channels as optional iio interfaces
>>> as weel. This can be used for debugging or special applications.
>>>
>>> This patch also splits the tsc2007 driver in several source files:
>>> tsc2007.h -- constants, structs and stubs
>>> tsc2007_core.c -- functional parts of the original driver
>>> tsc2007_iio.c -- the optional iio stuff
>>>
>>> Makefile magic allows to conditionally link the iio stuff
>>> if CONFIG_IIO=y or =m in a way that it works with
>>> CONFIG_TOUCHSCREEN_TSC2007=m.
>>>
>>> Signed-off-by: H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
>>> Reviewed-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>> ---
>>> drivers/input/touchscreen/Makefile | 7 +
>>> drivers/input/touchscreen/tsc2007.h | 116
>>> ++++++++++++++++
>>> .../touchscreen/{tsc2007.c => tsc2007_core.c} | 95 +++----------
>>> drivers/input/touchscreen/tsc2007_iio.c | 150
>>> +++++++++++++++++++++
>>> 4 files changed, 294 insertions(+), 74 deletions(-)
>>> create mode 100644 drivers/input/touchscreen/tsc2007.h
>>> rename drivers/input/touchscreen/{tsc2007.c => tsc2007_core.c} (86%)
>>> create mode 100644 drivers/input/touchscreen/tsc2007_iio.c
>>>
>>> diff --git a/drivers/input/touchscreen/Makefile
>>> b/drivers/input/touchscreen/Makefile
>>> index 81b8645..3be0d19 100644
>>> --- a/drivers/input/touchscreen/Makefile
>>> +++ b/drivers/input/touchscreen/Makefile
>>> @@ -80,6 +80,13 @@ obj-$(CONFIG_TOUCHSCREEN_TSC_SERIO) += tsc40.o
>>> obj-$(CONFIG_TOUCHSCREEN_TSC200X_CORE) += tsc200x-core.o
>>> obj-$(CONFIG_TOUCHSCREEN_TSC2004) += tsc2004.o
>>> obj-$(CONFIG_TOUCHSCREEN_TSC2005) += tsc2005.o
>>> +tsc2007-y := tsc2007_core.o
>>> +ifeq ($(CONFIG_IIO),y)
>>> +tsc2007-y += tsc2007_iio.o
>>> +endif
>>> +ifeq ($(CONFIG_IIO),m)
>>> +tsc2007-y += tsc2007_iio.o
>>
>> Not tsc2007-m ?
>>
>> I don't follow how this works!
>
> I guess tsc2007-y is an internal collector variable name
> for multiple .o components. Sort of a "library" object.
>
> While
>
> obj-y += tsc2007.o adds it to the kernel
> obj-m += tsc2007.o adds it to the modules
>
> I am not sure if my explanation is correct but it appears
> to work that way.
>
> Anyways what shall we do? If CONFIG_TOUCHSCREEN_TSC2007=y
> and IIO=m we have a problem that we need dynamic binding.
Yes, we just need to block that particular combination. Only build in the IIO support
if it is also built in. That's way I thought we'd want to add it tsc2007-m which would
only be used if tsc2007 as a whole was built as a module.
Otherwise it would be ignored (I think!)
I'm not seeing this structure anywhere else in kernel - hence cc'd Yann and the Kbuild list
to see if they can offer some advices.
As a quick summary, we are looking to add IIO support to this driver in the following circumstances.
IIO and this driver are modules. (ideally handling the dependencies nicely)
IIO and this driver are both built in.
Problem case is driver built in and IIO as a module.
Jonathan
>
>>> +endif
>>> obj-$(CONFIG_TOUCHSCREEN_TSC2007) += tsc2007.o
>>> obj-$(CONFIG_TOUCHSCREEN_UCB1400) += ucb1400_ts.o
>>> obj-$(CONFIG_TOUCHSCREEN_WACOM_W8001) += wacom_w8001.o
>>> diff --git a/drivers/input/touchscreen/tsc2007.h
>>> b/drivers/input/touchscreen/tsc2007.h
>>> new file mode 100644
>>> index 0000000..c25932f
>>> --- /dev/null
>>> +++ b/drivers/input/touchscreen/tsc2007.h
>>> @@ -0,0 +1,116 @@
>>> +/*
>>> + * Copyright (c) 2008 MtekVision Co., Ltd.
>>> + * Kwangwoo Lee <kwlee-ec7hoAtq5SbSUeElwK9/Pw@public.gmane.org>
>>> + *
>>> + * Using code from:
>>> + * - ads7846.c
>>> + * Copyright (c) 2005 David Brownell
>>> + * Copyright (c) 2006 Nokia Corporation
>>> + * - corgi_ts.c
>>> + * Copyright (C) 2004-2005 Richard Purdie
>>> + * - omap_ts.[hc], ads7846.h, ts_osk.c
>>> + * Copyright (C) 2002 MontaVista Software
>>> + * Copyright (C) 2004 Texas Instruments
>>> + * Copyright (C) 2005 Dirk Behme
>>> + *
>>> + * This program is free software; you can redistribute it and/or
>>> modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +#include <linux/input/touchscreen.h>
>>> +
>>> +#define TSC2007_MEASURE_TEMP0 (0x0 << 4)
>>> +#define TSC2007_MEASURE_AUX (0x2 << 4)
>>> +#define TSC2007_MEASURE_TEMP1 (0x4 << 4)
>>> +#define TSC2007_ACTIVATE_XN (0x8 << 4)
>>> +#define TSC2007_ACTIVATE_YN (0x9 << 4)
>>> +#define TSC2007_ACTIVATE_YP_XN (0xa << 4)
>>> +#define TSC2007_SETUP (0xb << 4)
>>> +#define TSC2007_MEASURE_X (0xc << 4)
>>> +#define TSC2007_MEASURE_Y (0xd << 4)
>>> +#define TSC2007_MEASURE_Z1 (0xe << 4)
>>> +#define TSC2007_MEASURE_Z2 (0xf << 4)
>>> +
>>> +#define TSC2007_POWER_OFF_IRQ_EN (0x0 << 2)
>>> +#define TSC2007_ADC_ON_IRQ_DIS0 (0x1 << 2)
>>> +#define TSC2007_ADC_OFF_IRQ_EN (0x2 << 2)
>>> +#define TSC2007_ADC_ON_IRQ_DIS1 (0x3 << 2)
>>> +
>>> +#define TSC2007_12BIT (0x0 << 1)
>>> +#define TSC2007_8BIT (0x1 << 1)
>>> +
>>> +#define MAX_12BIT ((1 << 12) - 1)
>>> +
>>> +#define ADC_ON_12BIT (TSC2007_12BIT | TSC2007_ADC_ON_IRQ_DIS0)
>>> +
>>> +#define READ_Y (ADC_ON_12BIT | TSC2007_MEASURE_Y)
>>> +#define READ_Z1 (ADC_ON_12BIT | TSC2007_MEASURE_Z1)
>>> +#define READ_Z2 (ADC_ON_12BIT | TSC2007_MEASURE_Z2)
>>> +#define READ_X (ADC_ON_12BIT | TSC2007_MEASURE_X)
>>> +#define PWRDOWN (TSC2007_12BIT | TSC2007_POWER_OFF_IRQ_EN)
>>> +
>>> +struct ts_event {
>>> + u16 x;
>>> + u16 y;
>>> + u16 z1, z2;
>>> +};
>>> +
>>> +struct tsc2007 {
>>> + struct input_dev *input;
>>> + char phys[32];
>>> +
>>> + struct i2c_client *client;
>>> +
>>> + u16 model;
>>> + u16 x_plate_ohms;
>>> +
>>> + struct touchscreen_properties prop;
>>> +
>>> + bool report_resistance;
>>> + u16 min_x;
>>> + u16 min_y;
>>> + u16 max_x;
>>> + u16 max_y;
>>> + u16 max_rt;
>>> + unsigned long poll_period; /* in jiffies */
>>> + int fuzzx;
>>> + int fuzzy;
>>> + int fuzzz;
>>> +
>>> + unsigned int gpio;
>>> + int irq;
>>> +
>>> + wait_queue_head_t wait;
>>> + bool stopped;
>>> + bool pendown;
>>> +
>>> + int (*get_pendown_state)(struct device *);
>>> + void (*clear_penirq)(void);
>>> +
>>> + struct mutex mlock;
>>> + struct iio_dev *iio_dev; /* optional */
>>> +};
>>> +
>>> +int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd);
>>> +u32 tsc2007_calculate_resistance(struct tsc2007 *tsc,
>>> + struct ts_event *tc);
>>> +bool tsc2007_is_pen_down(struct tsc2007 *ts);
>>> +
>>> +#if IS_ENABLED(CONFIG_IIO)
>>> +
>>> +/* defined in tsc2007_iio.c */
>>> +int tsc2007_iio_configure(struct tsc2007 *ts);
>>> +void tsc2007_iio_unconfigure(struct tsc2007 *ts);
>>> +
>>> +#else /* CONFIG_IIO */
>>> +
>>> +static inline int tsc2007_iio_configure(struct tsc2007 *ts)
>>> +{
>>> + return 0;
>>> +}
>>> +static inline void tsc2007_iio_unconfigure(struct tsc2007 *ts)
>>> +{
>>> +}
>>> +
>>> +#endif /* CONFIG_IIO */
>>> diff --git a/drivers/input/touchscreen/tsc2007.c
>>> b/drivers/input/touchscreen/tsc2007_core.c
>>> similarity index 86%
>>> rename from drivers/input/touchscreen/tsc2007.c
>>> rename to drivers/input/touchscreen/tsc2007_core.c
>>> index 76b462b..812ded8 100644
>>> --- a/drivers/input/touchscreen/tsc2007.c
>>> +++ b/drivers/input/touchscreen/tsc2007_core.c
>>> @@ -27,79 +27,11 @@
>>> #include <linux/i2c.h>
>>> #include <linux/i2c/tsc2007.h>
>>> #include <linux/of_device.h>
>>> -#include <linux/of.h>
>>> #include <linux/of_gpio.h>
>>> -#include <linux/input/touchscreen.h>
>>> -
>>> -#define TSC2007_MEASURE_TEMP0 (0x0 << 4)
>>> -#define TSC2007_MEASURE_AUX (0x2 << 4)
>>> -#define TSC2007_MEASURE_TEMP1 (0x4 << 4)
>>> -#define TSC2007_ACTIVATE_XN (0x8 << 4)
>>> -#define TSC2007_ACTIVATE_YN (0x9 << 4)
>>> -#define TSC2007_ACTIVATE_YP_XN (0xa << 4)
>>> -#define TSC2007_SETUP (0xb << 4)
>>> -#define TSC2007_MEASURE_X (0xc << 4)
>>> -#define TSC2007_MEASURE_Y (0xd << 4)
>>> -#define TSC2007_MEASURE_Z1 (0xe << 4)
>>> -#define TSC2007_MEASURE_Z2 (0xf << 4)
>>> -
>>> -#define TSC2007_POWER_OFF_IRQ_EN (0x0 << 2)
>>> -#define TSC2007_ADC_ON_IRQ_DIS0 (0x1 << 2)
>>> -#define TSC2007_ADC_OFF_IRQ_EN (0x2 << 2)
>>> -#define TSC2007_ADC_ON_IRQ_DIS1 (0x3 << 2)
>>> -
>>> -#define TSC2007_12BIT (0x0 << 1)
>>> -#define TSC2007_8BIT (0x1 << 1)
>>> -
>>> -#define MAX_12BIT ((1 << 12) - 1)
>>> -
>>> -#define ADC_ON_12BIT (TSC2007_12BIT | TSC2007_ADC_ON_IRQ_DIS0)
>>> -
>>> -#define READ_Y (ADC_ON_12BIT | TSC2007_MEASURE_Y)
>>> -#define READ_Z1 (ADC_ON_12BIT | TSC2007_MEASURE_Z1)
>>> -#define READ_Z2 (ADC_ON_12BIT | TSC2007_MEASURE_Z2)
>>> -#define READ_X (ADC_ON_12BIT | TSC2007_MEASURE_X)
>>> -#define PWRDOWN (TSC2007_12BIT | TSC2007_POWER_OFF_IRQ_EN)
>>> -
>>> -struct ts_event {
>>> - u16 x;
>>> - u16 y;
>>> - u16 z1, z2;
>>> -};
>>> -
>>> -struct tsc2007 {
>>> - struct input_dev *input;
>>> - char phys[32];
>>> -
>>> - struct i2c_client *client;
>>> -
>>> - u16 model;
>>> - u16 x_plate_ohms;
>>> -
>>> - struct touchscreen_properties prop;
>>> -
>>> - bool report_resistance;
>>> - u16 min_x;
>>> - u16 min_y;
>>> - u16 max_x;
>>> - u16 max_y;
>>> - u16 max_rt;
>>> - unsigned long poll_period; /* in jiffies */
>>> - int fuzzx;
>>> - int fuzzy;
>>> - int fuzzz;
>>> -
>>> - unsigned gpio;
>>> - int irq;
>>> -
>>> - wait_queue_head_t wait;
>>> - bool stopped;
>>> +#include "tsc2007.h"
>>>
>>> - int (*get_pendown_state)(struct device *);
>>> - void (*clear_penirq)(void);
>>> -};
>>>
>>> -static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
>>> +int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
>>> {
>>> s32 data;
>>> u16 val;
>>> @@ -137,7 +69,7 @@ static void tsc2007_read_values(struct tsc2007 *tsc,
>>> struct ts_event *tc)
>>> tsc2007_xfer(tsc, PWRDOWN);
>>> }
>>>
>>> -static u32 tsc2007_calculate_resistance(struct tsc2007 *tsc,
>>> +u32 tsc2007_calculate_resistance(struct tsc2007 *tsc,
>>> struct ts_event *tc)
>>> {
>>> u32 rt = 0;
>>> @@ -158,7 +90,7 @@ static u32 tsc2007_calculate_resistance(struct
>>> tsc2007 *tsc,
>>> return rt;
>>> }
>>>
>>> -static bool tsc2007_is_pen_down(struct tsc2007 *ts)
>>> +bool tsc2007_is_pen_down(struct tsc2007 *ts)
>>> {
>>> /*
>>> * NOTE: We can't rely on the pressure to determine the pen down
>>> @@ -191,7 +123,10 @@ static irqreturn_t tsc2007_soft_irq(int irq, void
>>> *handle)
>>> while (!ts->stopped && tsc2007_is_pen_down(ts)) {
>>>
>>> /* pen is down, continue with the measurement */
>>> +
>>> + mutex_lock(&ts->mlock);
>>> tsc2007_read_values(ts, &tc);
>>> + mutex_unlock(&ts->mlock);
>>>
>>> rt = tsc2007_calculate_resistance(ts, &tc);
>>>
>>> @@ -441,7 +376,8 @@ static void tsc2007_call_exit_platform_hw(void
>>> *data)
>>> static int tsc2007_probe(struct i2c_client *client,
>>> const struct i2c_device_id *id)
>>> {
>>> - const struct tsc2007_platform_data *pdata =
>>> dev_get_platdata(&client->dev);
>>> + const struct tsc2007_platform_data *pdata =
>>> + dev_get_platdata(&client->dev);
>>> struct tsc2007 *ts;
>>> struct input_dev *input_dev;
>>> int err;
>>> @@ -463,7 +399,9 @@ static int tsc2007_probe(struct i2c_client *client,
>>> ts->client = client;
>>> ts->irq = client->irq;
>>> ts->input = input_dev;
>>> +
>>> init_waitqueue_head(&ts->wait);
>>> + mutex_init(&ts->mlock);
>>>
>>> snprintf(ts->phys, sizeof(ts->phys),
>>> "%s/input0", dev_name(&client->dev));
>>> @@ -534,7 +472,7 @@ static int tsc2007_probe(struct i2c_client *client,
>>> if (err < 0) {
>>> dev_err(&client->dev,
>>> "Failed to setup chip: %d\n", err);
>>> - return err; /* usually, chip does not respond */
>>> + return err; /* chip does not respond */
>>> }
>>>
>>> err = input_register_device(input_dev);
>>> @@ -544,6 +482,14 @@ static int tsc2007_probe(struct i2c_client
>>> *client,
>>> return err;
>>> }
>>>
>>> + return tsc2007_iio_configure(ts);
>>> +}
>>> +
>>> +static int tsc2007_remove(struct i2c_client *client)
>>> +{
>>> + struct tsc2007 *ts = i2c_get_clientdata(client);
>>> +
>>> + tsc2007_iio_unconfigure(ts);
>>> return 0;
>>> }
>>>
>>> @@ -569,6 +515,7 @@ static struct i2c_driver tsc2007_driver = {
>>> },
>>> .id_table = tsc2007_idtable,
>>> .probe = tsc2007_probe,
>>> + .remove = tsc2007_remove,
>>> };
>>>
>>> module_i2c_driver(tsc2007_driver);
>>> diff --git a/drivers/input/touchscreen/tsc2007_iio.c
>>> b/drivers/input/touchscreen/tsc2007_iio.c
>>> new file mode 100644
>>> index 0000000..ed79944
>>> --- /dev/null
>>> +++ b/drivers/input/touchscreen/tsc2007_iio.c
>>> @@ -0,0 +1,150 @@
>>> +/*
>>> + * Copyright (c) 2016 Golden Delicious Comp. GmbH&Co. KG
>>> + * Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@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 version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +#include <linux/i2c.h>
>>> +#include <linux/iio/iio.h>
>>> +#include "tsc2007.h"
>>> +
>>> +struct tsc2007_iio {
>>> + struct tsc2007 *ts;
>>> +};
>>> +
>>> +#define TSC2007_CHAN_IIO(_chan, _name, _type, _chan_info) \
>>> +{ \
>>> + .datasheet_name = _name, \
>>> + .type = _type, \
>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
>>> + BIT(_chan_info), \
>>> + .indexed = 1, \
>>> + .channel = _chan, \
>>> +}
>>> +
>>> +static const struct iio_chan_spec tsc2007_iio_channel[] = {
>>> + TSC2007_CHAN_IIO(0, "x", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
>>> + TSC2007_CHAN_IIO(1, "y", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
>>> + TSC2007_CHAN_IIO(2, "z1", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
>>> + TSC2007_CHAN_IIO(3, "z2", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
>>> + TSC2007_CHAN_IIO(4, "adc", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
>>> + TSC2007_CHAN_IIO(5, "rt", IIO_VOLTAGE, IIO_CHAN_INFO_RAW), /* Ohms?
>>> */
>>> + TSC2007_CHAN_IIO(6, "pen", IIO_PRESSURE, IIO_CHAN_INFO_RAW),
>>> + TSC2007_CHAN_IIO(7, "temp0", IIO_TEMP, IIO_CHAN_INFO_RAW),
>>> + TSC2007_CHAN_IIO(8, "temp1", IIO_TEMP, IIO_CHAN_INFO_RAW),
>>> +};
>>> +
>>> +static int tsc2007_read_raw(struct iio_dev *indio_dev,
>>> + struct iio_chan_spec const *chan, int *val, int *val2, long mask)
>>> +{
>>> + struct tsc2007_iio *iio = iio_priv(indio_dev);
>>> + struct tsc2007 *tsc = iio->ts;
>>> + int adc_chan = chan->channel;
>>> + int ret = 0;
>>> +
>>> + if (adc_chan >= ARRAY_SIZE(tsc2007_iio_channel))
>>> + return -EINVAL;
>>> +
>>> + if (mask != IIO_CHAN_INFO_RAW)
>>> + return -EINVAL;
>>> +
>>> + mutex_lock(&tsc->mlock);
>>> +
>>> + switch (chan->channel) {
>>> + case 0:
>>> + *val = tsc2007_xfer(tsc, READ_X);
>>> + break;
>>> + case 1:
>>> + *val = tsc2007_xfer(tsc, READ_Y);
>>> + break;
>>> + case 2:
>>> + *val = tsc2007_xfer(tsc, READ_Z1);
>>> + break;
>>> + case 3:
>>> + *val = tsc2007_xfer(tsc, READ_Z2);
>>> + break;
>>> + case 4:
>>> + *val = tsc2007_xfer(tsc, (ADC_ON_12BIT | TSC2007_MEASURE_AUX));
>>> + break;
>>> + case 5: {
>>> + struct ts_event tc;
>>> +
>>> + tc.x = tsc2007_xfer(tsc, READ_X);
>>> + tc.z1 = tsc2007_xfer(tsc, READ_Z1);
>>> + tc.z2 = tsc2007_xfer(tsc, READ_Z2);
>>> + *val = tsc2007_calculate_resistance(tsc, &tc);
>>> + break;
>>> + }
>>> + case 6:
>>> + *val = tsc2007_is_pen_down(tsc);
>>> + break;
>>> + case 7:
>>> + *val = tsc2007_xfer(tsc,
>>> + (ADC_ON_12BIT | TSC2007_MEASURE_TEMP0));
>>> + break;
>>> + case 8:
>>> + *val = tsc2007_xfer(tsc,
>>> + (ADC_ON_12BIT | TSC2007_MEASURE_TEMP1));
>>> + break;
>>> + }
>>> +
>>> + /* Prepare for next touch reading - power down ADC, enable PENIRQ */
>>> + tsc2007_xfer(tsc, PWRDOWN);
>>> +
>>> + mutex_unlock(&tsc->mlock);
>>> +
>>> + ret = IIO_VAL_INT;
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static const struct iio_info tsc2007_iio_info = {
>>> + .read_raw = tsc2007_read_raw,
>>> + .driver_module = THIS_MODULE,
>>> +};
>>> +
>>> +int tsc2007_iio_configure(struct tsc2007 *ts)
>>> +{
>>> + int err;
>>> + struct iio_dev *indio_dev;
>>> + struct tsc2007_iio *iio;
>>> +
>>> + indio_dev = devm_iio_device_alloc(&ts->client->dev,
>>> + sizeof(struct tsc2007_iio));
>>> + if (!indio_dev) {
>>> + dev_err(&ts->client->dev, "iio_device_alloc failed\n");
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + iio = iio_priv(indio_dev);
>>> + iio->ts = ts;
>>> + ts->iio_dev = (void *) indio_dev;
>>> +
>>> + indio_dev->name = "tsc2007";
>>> + indio_dev->dev.parent = &ts->client->dev;
>>> + indio_dev->info = &tsc2007_iio_info;
>>> + indio_dev->modes = INDIO_DIRECT_MODE;
>>> + indio_dev->channels = tsc2007_iio_channel;
>>> + indio_dev->num_channels = ARRAY_SIZE(tsc2007_iio_channel);
>>> +
>>> + err = iio_device_register(indio_dev);
>>> + if (err < 0) {
>>> + dev_err(&ts->client->dev, "iio_device_register() failed: %d\n",
>>> + err);
>>> + return err;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL(tsc2007_iio_configure);
>>> +
>>> +void tsc2007_iio_unconfigure(struct tsc2007 *ts)
>>> +{
>>> + struct iio_dev *indio_dev = ts->iio_dev;
>>> +
>>> + iio_device_unregister(indio_dev);
>>> +}
>>> +EXPORT_SYMBOL(tsc2007_iio_unconfigure);
>>
>> --
>> Sent from my Android device with K-9 Mail. Please excuse my brevity.
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox