Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH v3 1/3] phy: phy-core: allow specifying supply at port level
From: Arun Ramamurthy @ 2015-04-22 23:14 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Russell King, Kishon Vijay Abraham I, Gregory CLEMENT,
	Gabriel FERNANDEZ, Jason Cooper, Thomas Petazzoni,
	Greg Kroah-Hartman, Arnd Bergmann
  Cc: devicetree, linux-kernel, linux-fbdev, Dmitry Torokhov,
	Anatol Pomazau, Jonathan Richardson, Scott Branden, Ray Jui,
	bcm-kernel-feedback-list, Dmitry Torokhov, Arun Ramamurthy
In-Reply-To: <1429744479-10410-1-git-send-email-arun.ramamurthy@broadcom.com>

From: Dmitry Torokhov <dtor@chromium.org>

Multi-port phys may have per port power supplies. Let's change phy
core to look for supply at the port level when multiple ports are
specified. To keep compatibility with the existing device tree board
descriptions for single-port phys we will continue looking up the
power supply at the parent node level

Signed-off-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>
Reviewed-by: Ray Jui <rjui@broadcom.com>
Reviewed-by: Scott Branden <sbranden@broadcom.com>
---
 drivers/phy/phy-core.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 3791838..c7aa297 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -651,16 +651,6 @@ struct phy *phy_create(struct device *dev, struct device_node *node,
 		goto free_phy;
 	}
 
-	/* phy-supply */
-	phy->pwr = regulator_get_optional(dev, "phy");
-	if (IS_ERR(phy->pwr)) {
-		if (PTR_ERR(phy->pwr) = -EPROBE_DEFER) {
-			ret = -EPROBE_DEFER;
-			goto free_ida;
-		}
-		phy->pwr = NULL;
-	}
-
 	device_initialize(&phy->dev);
 	mutex_init(&phy->mutex);
 
@@ -674,6 +664,16 @@ struct phy *phy_create(struct device *dev, struct device_node *node,
 	if (ret)
 		goto put_dev;
 
+	/* phy-supply */
+	phy->pwr = regulator_get_optional(&phy->dev, "phy");
+	if (IS_ERR(phy->pwr)) {
+		ret = PTR_ERR(phy->pwr);
+		if (ret = -EPROBE_DEFER)
+			goto put_dev;
+
+		phy->pwr = NULL;
+	}
+
 	ret = device_add(&phy->dev);
 	if (ret)
 		goto put_dev;
@@ -689,9 +689,6 @@ put_dev:
 	put_device(&phy->dev);  /* calls phy_release() which frees resources */
 	return ERR_PTR(ret);
 
-free_ida:
-	ida_simple_remove(&phy_ida, phy->id);
-
 free_phy:
 	kfree(phy);
 	return ERR_PTR(ret);
-- 
2.3.4


^ permalink raw reply related

* [PATCH v3 2/3] Phy: DT binding documentation for Broadcom Cygnus USB PHY driver
From: Arun Ramamurthy @ 2015-04-22 23:14 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Russell King, Kishon Vijay Abraham I, Gregory CLEMENT,
	Gabriel FERNANDEZ, Jason Cooper, Thomas Petazzoni,
	Greg Kroah-Hartman, Arnd Bergmann
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Dmitry Torokhov,
	Anatol Pomazau, Jonathan Richardson, Scott Branden, Ray Jui,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Arun Ramamurthy
In-Reply-To: <1429744479-10410-1-git-send-email-arun.ramamurthy-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

Broadcom's Cygnus chip has a USB 2.0 host controller connected to
three separate phys. One of the phs (port 2) is also connectd to
a usb 2.0 device controller

Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>
Reviewed-by: Ray Jui <rjui@broadcom.com>
Reviewed-by: Scott Branden <sbranden@broadcom.com>
---
 .../bindings/phy/brcm,cygnus-usb-phy.txt           | 69 ++++++++++++++++++++++
 include/dt-bindings/phy/phy.h                      |  2 +
 2 files changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/brcm,cygnus-usb-phy.txt

diff --git a/Documentation/devicetree/bindings/phy/brcm,cygnus-usb-phy.txt b/Documentation/devicetree/bindings/phy/brcm,cygnus-usb-phy.txt
new file mode 100644
index 0000000..ec62044
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/brcm,cygnus-usb-phy.txt
@@ -0,0 +1,69 @@
+BROADCOM CYGNUS USB PHY
+
+Required Properties:
+	- compatible:  brcm,cygnus-usb-phy
+	- reg : usbphy_regs - Base address of phy registers
+			usb2h_idm_regs - Base address of host idm registers
+			usb2d_idm_regs - Base address of device idm registers
+	- phy-cells - must be 1 for each port declared. The node
+		      that uses the phy must provide either PHY_CONFIG_DEVICE for device
+		      or PHY_CONFIG_HOST for host
+
+NOTE: port 0 and port 1 are host only and port 2 can be configured for host or
+device.
+
+Example of phy :
+	usbphy0: usbphy@0x0301c000 {
+		compatible = "brcm,cygnus-usb-phy";
+		reg = <0x0301c000 0x2000>,
+		      <0x18115000 0x1000>,
+		      <0x18111000 0x1000>;
+		status = "okay";
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+		usbphy0_0: usbphy0@0 {
+			#phy-cells = <1>;
+			reg = <0>;
+			status = "okay";
+			phy-supply = <&vbus_p0>;
+		};
+
+		usbphy0_1: usbphy0@1 {
+			#phy-cells = <1>;
+			reg = <1>;
+			status = "okay";
+		};
+
+		usbphy0_2: usbphy0@2 {
+			#phy-cells = <1>;
+			reg = <2>;
+			status = "okay";
+			phy-supply = <&vbus_p2>;
+		};
+	};
+
+Example of node using the phy:
+
+	/* This nodes declares all three ports as host */
+
+	ehci0: usb@0x18048000 {
+		compatible = "generic-ehci";
+		reg = <0x18048000 0x100>;
+		interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
+		phys = <&usbphy0_0 PHY_CONFIG_HOST &usbphy0_1 PHY_CONFIG_HOST &usbphy0_2 PHY_CONFIG_HOST>;
+		status = "okay";
+	};
+
+	/*
+	 * This node declares port 2 phy
+	 * and configures it for device
+	 */
+
+	usbd_udc_dwc1: usbd_udc_dwc@0x1804c000 {
+		compatible = "iproc-udc";
+		reg = <0x1804c000 0x2000>;
+		interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
+		phys = <&usbphy0_2 PHY_CONFIG_DEVICE>;
+		phy-names = "usb";
+	};
diff --git a/include/dt-bindings/phy/phy.h b/include/dt-bindings/phy/phy.h
index 6c90193..3f6b1ac 100644
--- a/include/dt-bindings/phy/phy.h
+++ b/include/dt-bindings/phy/phy.h
@@ -15,5 +15,7 @@
 #define PHY_TYPE_PCIE		2
 #define PHY_TYPE_USB2		3
 #define PHY_TYPE_USB3		4
+#define PHY_CONFIG_HOST		1
+#define PHY_CONFIG_DEVICE	0
 
 #endif /* _DT_BINDINGS_PHY */
-- 
2.3.4


^ permalink raw reply related

* [PATCH v3 3/3] phy: cygnus-usbphy: Add Broadcom Cygnus USB phy driver
From: Arun Ramamurthy @ 2015-04-22 23:14 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Russell King, Kishon Vijay Abraham I, Gregory CLEMENT,
	Gabriel FERNANDEZ, Jason Cooper, Thomas Petazzoni,
	Greg Kroah-Hartman, Arnd Bergmann
  Cc: devicetree, linux-kernel, linux-fbdev, Dmitry Torokhov,
	Anatol Pomazau, Jonathan Richardson, Scott Branden, Ray Jui,
	bcm-kernel-feedback-list, Arun Ramamurthy, Dmitry Torokhov
In-Reply-To: <1429744479-10410-1-git-send-email-arun.ramamurthy@broadcom.com>

This driver adds support for USB 2.0 host and device phy for
Broadcom's Cygnus chipset. The host controller is connected to
three separate phys and one of the phys (port 2) is connected to
the device controller

Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com>
Signed-off-by: Dmitry Torokhov <dtor@chromium.org>
Reviewed-by: Ray Jui <rjui@broadcom.com>
Reviewed-by: Scott Branden <sbranden@broadcom.com>
---
 drivers/phy/Kconfig              |  12 +
 drivers/phy/Makefile             |   1 +
 drivers/phy/phy-bcm-cygnus-usb.c | 504 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 517 insertions(+)
 create mode 100644 drivers/phy/phy-bcm-cygnus-usb.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 2962de2..0aa62dc 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -291,4 +291,16 @@ config PHY_QCOM_UFS
 	help
 	  Support for UFS PHY on QCOM chipsets.
 
+config PHY_BCM_CYGNUS_USB
+	tristate "Broadcom Cygnus USB PHY support"
+	depends on OF
+	depends on ARCH_BCM_CYGNUS || COMPILE_TEST
+	select GENERIC_PHY
+	default ARCH_BCM_CYGNUS
+	help
+	  Enable this to support the USB PHY in Broadcom's Cygnus chip.
+	  The phys are capable of supporting host mode for all ports
+	  and device mode for port 2. The host or device configuration is
+	  read from the device tree.
+
 endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index f080e1b..47a37fa 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
 obj-$(CONFIG_PHY_BERLIN_USB)		+= phy-berlin-usb.o
 obj-$(CONFIG_PHY_BERLIN_SATA)		+= phy-berlin-sata.o
 obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY)	+= phy-armada375-usb2.o
+obj-$(CONFIG_PHY_BCM_CYGNUS_USB)	+= phy-bcm-cygnus-usb.o
 obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
 obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
 obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
diff --git a/drivers/phy/phy-bcm-cygnus-usb.c b/drivers/phy/phy-bcm-cygnus-usb.c
new file mode 100644
index 0000000..e718404
--- /dev/null
+++ b/drivers/phy/phy-bcm-cygnus-usb.c
@@ -0,0 +1,504 @@
+/*
+ * Copyright (C) 2015 Broadcom Corporation
+ *
+ * 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 version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/phy/phy.h>
+#include <linux/delay.h>
+
+#define CDRU_USBPHY_CLK_RST_SEL_OFFSET			0x11b4
+#define CDRU_USBPHY2_HOST_DEV_SEL_OFFSET		0x11b8
+#define CDRU_SPARE_REG_0_OFFSET				0x1238
+#define CRMU_USB_PHY_AON_CTRL_OFFSET			0x00028
+#define CDRU_USB_DEV_SUSPEND_RESUME_CTRL_OFFSET		0x1210
+#define CDRU_USBPHY_P0_STATUS_OFFSET			0x11D0
+#define CDRU_USBPHY_P1_STATUS_OFFSET			0x11E8
+#define CDRU_USBPHY_P2_STATUS_OFFSET			0x1200
+#define CDRU_USBPHY_USBPHY_ILDO_ON_FLAG			1
+#define CDRU_USBPHY_USBPHY_PLL_LOCK			0
+#define CDRU_USB_DEV_SUSPEND_RESUME_CTRL_DISABLE	0
+#define PHY2_DEV_HOST_CTRL_SEL_HOST			1
+
+#define CRMU_USBPHY_P0_AFE_CORERDY_VDDC			1
+#define CRMU_USBPHY_P0_RESETB				2
+#define CRMU_USBPHY_P1_AFE_CORERDY_VDDC			9
+#define CRMU_USBPHY_P1_RESETB				10
+#define CRMU_USBPHY_P2_AFE_CORERDY_VDDC			17
+#define CRMU_USBPHY_P2_RESETB				18
+
+#define USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET		0x0408
+#define USB2_IDM_IDM_IO_CONTROL_DIRECT__clk_enable	0
+#define USB2_IDM_IDM_RESET_CONTROL_OFFSET		0x0800
+#define USB2_IDM_IDM_RESET_CONTROL__RESET		0
+
+#define PLL_LOCK_RETRY_COUNT	1000
+#define MAX_PHY_PORTS		3
+
+static int power_bit[] = {CRMU_USBPHY_P0_AFE_CORERDY_VDDC,
+				CRMU_USBPHY_P1_AFE_CORERDY_VDDC,
+				CRMU_USBPHY_P2_AFE_CORERDY_VDDC};
+static int reset_bit[] = {CRMU_USBPHY_P0_RESETB,
+				CRMU_USBPHY_P1_RESETB,
+				CRMU_USBPHY_P2_RESETB};
+static int status_reg[] = {CDRU_USBPHY_P0_STATUS_OFFSET,
+				CDRU_USBPHY_P1_STATUS_OFFSET,
+				CDRU_USBPHY_P2_STATUS_OFFSET};
+
+struct bcm_phy_instance;
+
+struct bcm_phy_driver {
+	void __iomem *usbphy_regs;
+	void __iomem *usb2h_idm_regs;
+	void __iomem *usb2d_idm_regs;
+	struct bcm_phy_instance *ports[MAX_PHY_PORTS];
+	spinlock_t lock;
+	bool idm_host_enabled;
+};
+
+struct bcm_phy_instance {
+	struct bcm_phy_driver *driver;
+	struct phy *generic_phy;
+	int port_no;
+	bool host_mode; /* true - Host, false - device */
+	bool power;
+};
+
+static inline int bcm_phy_cdru_usbphy_status_wait(u32 usb_reg, int reg_bit,
+					  struct bcm_phy_driver *phy_driver)
+{
+	/* Wait for the PLL lock status */
+	int retry = PLL_LOCK_RETRY_COUNT;
+	u32 reg_val;
+
+	do {
+		udelay(1);
+		reg_val = readl(phy_driver->usbphy_regs +
+				usb_reg);
+		if (reg_val & (1 << reg_bit))
+			return 0;
+	} while (--retry > 0);
+
+	return -EBUSY;
+
+}
+
+static struct phy *bcm_usb_phy_xlate(struct device *dev,
+				     struct of_phandle_args *args)
+{
+	struct bcm_phy_driver *phy_driver = dev_get_drvdata(dev);
+	struct bcm_phy_instance *port = NULL;
+	int i;
+
+	if (!phy_driver)
+		return ERR_PTR(-ENODEV);
+
+	if (WARN_ON(args->args_count != 1))
+		return ERR_PTR(-ENODEV);
+
+	if (WARN_ON(args->args[0] < 0 || args->args[0] > 1))
+		return ERR_PTR(-ENODEV);
+
+	for (i = 0; i < ARRAY_SIZE(phy_driver->ports); i++) {
+		struct bcm_phy_instance *p = phy_driver->ports[i];
+
+		if (p && p->generic_phy->dev.of_node = args->np) {
+			port = p;
+			break;
+		}
+	}
+
+	if (!port) {
+		dev_err(dev, "Failed to locate phy %s\n", args->np->name);
+		return ERR_PTR(-EINVAL);
+	}
+
+	port->host_mode = args->args[0];
+
+	return port->generic_phy;
+}
+
+static int bcm_phy_init(struct phy *generic_phy)
+{
+	struct bcm_phy_instance *port = phy_get_drvdata(generic_phy);
+	struct bcm_phy_driver *phy_driver = port->driver;
+	unsigned long flags;
+	u32 reg_val;
+
+	spin_lock_irqsave(&phy_driver->lock, flags);
+
+	/*
+	 * Only PORT 2 is capable of being device and host
+	 * Default setting is device, check if it is set to host.
+	 */
+	if (port->port_no != 2) {
+		spin_unlock_irqrestore(&phy_driver->lock, flags);
+		return 0;
+	}
+
+	if (port->host_mode) {
+		writel(PHY2_DEV_HOST_CTRL_SEL_HOST,
+			phy_driver->usbphy_regs +
+			CDRU_USBPHY2_HOST_DEV_SEL_OFFSET);
+	} else {
+		/*
+		 * Disable suspend/resume signals to device controller
+		 * when a port is in device mode.
+		 */
+		reg_val = readl(phy_driver->usbphy_regs +
+			      CDRU_USB_DEV_SUSPEND_RESUME_CTRL_OFFSET);
+		reg_val |+			1 << CDRU_USB_DEV_SUSPEND_RESUME_CTRL_DISABLE;
+		writel(reg_val, phy_driver->usbphy_regs +
+		       CDRU_USB_DEV_SUSPEND_RESUME_CTRL_OFFSET);
+	}
+
+
+	spin_unlock_irqrestore(&phy_driver->lock, flags);
+	return 0;
+}
+
+static int bcm_phy_shutdown(struct phy *generic_phy)
+{
+	struct bcm_phy_instance *port = phy_get_drvdata(generic_phy);
+	struct bcm_phy_driver *phy_driver = port->driver;
+	unsigned long flags;
+	int i;
+	u32 reg_val, powered_on_phy;
+	bool power_off_flag = true;
+
+	spin_lock_irqsave(&phy_driver->lock, flags);
+
+	/* power down the phy */
+	reg_val = readl(phy_driver->usbphy_regs +
+			CRMU_USB_PHY_AON_CTRL_OFFSET);
+	reg_val &= ~(1 << power_bit[port->port_no]);
+	reg_val &= ~(1 << reset_bit[port->port_no]);
+	writel(reg_val, phy_driver->usbphy_regs +
+		CRMU_USB_PHY_AON_CTRL_OFFSET);
+	port->power = false;
+
+	/*
+	 * If a port is configured to device and it is being shutdown,
+	 * turn off the clocks to the usb device controller.
+	 */
+	if (port->port_no = 2 && !port->host_mode) {
+		reg_val = readl(phy_driver->usb2d_idm_regs +
+				USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET);
+		reg_val &= ~(1 << USB2_IDM_IDM_IO_CONTROL_DIRECT__clk_enable);
+		writel(reg_val, phy_driver->usb2d_idm_regs +
+				USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET);
+
+		reg_val = readl(phy_driver->usb2d_idm_regs +
+				 USB2_IDM_IDM_RESET_CONTROL_OFFSET);
+		reg_val |= 1 << USB2_IDM_IDM_RESET_CONTROL__RESET;
+		writel(reg_val, phy_driver->usb2d_idm_regs +
+				USB2_IDM_IDM_RESET_CONTROL_OFFSET);
+
+		spin_unlock_irqrestore(&phy_driver->lock, flags);
+		return 0;
+	}
+
+	/*
+	 * If the phy being shutdown provides clock and reset to
+	 * the host controller, change it do a different powered on phy
+	 * If all phys are powered off, shut of the host controller
+	 */
+	reg_val = readl(phy_driver->usbphy_regs +
+			CDRU_USBPHY_CLK_RST_SEL_OFFSET);
+	powered_on_phy = reg_val;
+	if (reg_val = port->port_no) {
+		for (i = 0; i < ARRAY_SIZE(phy_driver->ports); i++) {
+			if (phy_driver->ports[i] &&
+			    phy_driver->ports[i]->power &&
+			    phy_driver->ports[i]->host_mode) {
+				power_off_flag = false;
+				powered_on_phy = i;
+				break;
+			}
+		}
+	}
+
+	if (power_off_flag) {
+		/*
+		 * Put the host controller into reset state and
+		 * disable clock.
+		 */
+		reg_val = readl(phy_driver->usb2h_idm_regs +
+			USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET);
+		reg_val &+		  ~(1 << USB2_IDM_IDM_IO_CONTROL_DIRECT__clk_enable);
+		writel(reg_val, phy_driver->usb2h_idm_regs +
+				USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET);
+
+		reg_val = readl(phy_driver->usb2h_idm_regs +
+				 USB2_IDM_IDM_RESET_CONTROL_OFFSET);
+		reg_val |= (1 << USB2_IDM_IDM_RESET_CONTROL__RESET);
+		writel(reg_val, phy_driver->usb2h_idm_regs +
+				USB2_IDM_IDM_RESET_CONTROL_OFFSET);
+		phy_driver->idm_host_enabled = 0;
+	} else {
+		writel(powered_on_phy, phy_driver->usbphy_regs +
+			   CDRU_USBPHY_CLK_RST_SEL_OFFSET);
+	}
+
+	spin_unlock_irqrestore(&phy_driver->lock, flags);
+
+	return 0;
+}
+
+static int bcm_phy_poweron(struct phy *generic_phy)
+{
+	struct bcm_phy_instance *port = phy_get_drvdata(generic_phy);
+	struct bcm_phy_driver *phy_driver = port->driver;
+	unsigned long flags;
+	int ret;
+	u32 reg_val;
+	bool clock_reset_flag = true;
+
+	spin_lock_irqsave(&phy_driver->lock, flags);
+
+	/* Bring the AFE block out of reset to start powering up the PHY */
+	reg_val = readl(phy_driver->usbphy_regs + CRMU_USB_PHY_AON_CTRL_OFFSET);
+	reg_val |= 1 << power_bit[port->port_no];
+	writel(reg_val, phy_driver->usbphy_regs + CRMU_USB_PHY_AON_CTRL_OFFSET);
+
+	/* Check for power on and PLL lock */
+	ret = bcm_phy_cdru_usbphy_status_wait(status_reg[port->port_no],
+		   CDRU_USBPHY_USBPHY_ILDO_ON_FLAG, phy_driver);
+	if (ret < 0) {
+		dev_err(&generic_phy->dev,
+			"Timed out waiting for USBPHY_ILDO_ON_FLAG on port %d",
+			port->port_no);
+		goto err_shutdown;
+	}
+	ret = bcm_phy_cdru_usbphy_status_wait(status_reg[port->port_no],
+		CDRU_USBPHY_USBPHY_PLL_LOCK, phy_driver);
+	if (ret < 0) {
+		dev_err(&generic_phy->dev,
+			"Timed out waiting for USBPHY_PLL_LOCK on port %d",
+			port->port_no);
+		goto err_shutdown;
+	}
+	port->power = true;
+
+	/* Check if the port 2 is configured for device */
+	if (port->port_no = 2 && !port->host_mode) {
+		/*
+		 * Enable clock to USB device and get the USB device
+		 * out of reset.
+		 */
+		reg_val = readl(phy_driver->usb2d_idm_regs +
+				USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET);
+		reg_val |= 1 << USB2_IDM_IDM_IO_CONTROL_DIRECT__clk_enable;
+		writel(reg_val, phy_driver->usb2d_idm_regs +
+				USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET);
+
+		reg_val = readl(phy_driver->usb2d_idm_regs +
+				USB2_IDM_IDM_RESET_CONTROL_OFFSET);
+		reg_val &= ~(1 << USB2_IDM_IDM_RESET_CONTROL__RESET);
+		writel(reg_val, phy_driver->usb2d_idm_regs +
+				USB2_IDM_IDM_RESET_CONTROL_OFFSET);
+
+	} else {
+		reg_val = readl(phy_driver->usbphy_regs +
+				CDRU_USBPHY_CLK_RST_SEL_OFFSET);
+
+		/*
+		 * Check if the phy that is configured to provide clock
+		 * and reset is powered on.
+		 */
+		if (reg_val >= 0 && reg_val < ARRAY_SIZE(phy_driver->ports))
+			if (phy_driver->ports[reg_val])
+				if (phy_driver->ports[reg_val]->power)
+					clock_reset_flag = false;
+
+		/* if not set the current phy */
+		if (clock_reset_flag) {
+			reg_val = port->port_no;
+			writel(reg_val, phy_driver->usbphy_regs +
+			       CDRU_USBPHY_CLK_RST_SEL_OFFSET);
+		}
+	}
+
+	if (!phy_driver->idm_host_enabled) {
+		/* Enable clock to USB and get the USB out of reset */
+		reg_val = readl(phy_driver->usb2h_idm_regs +
+				USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET);
+		reg_val |= 1 << USB2_IDM_IDM_IO_CONTROL_DIRECT__clk_enable;
+		writel(reg_val, phy_driver->usb2h_idm_regs +
+				USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET);
+
+		reg_val = readl(phy_driver->usb2h_idm_regs +
+				USB2_IDM_IDM_RESET_CONTROL_OFFSET);
+		reg_val &= ~(1 << USB2_IDM_IDM_RESET_CONTROL__RESET);
+		writel(reg_val, phy_driver->usb2h_idm_regs +
+				USB2_IDM_IDM_RESET_CONTROL_OFFSET);
+		phy_driver->idm_host_enabled = true;
+	}
+
+	spin_unlock_irqrestore(&phy_driver->lock, flags);
+	return 0;
+
+err_shutdown:
+	spin_unlock_irqrestore(&phy_driver->lock, flags);
+	bcm_phy_shutdown(generic_phy);
+	return ret;
+}
+
+static struct phy_ops ops = {
+	.init		= bcm_phy_init,
+	.power_on	= bcm_phy_poweron,
+	.power_off	= bcm_phy_shutdown,
+};
+
+static const struct of_device_id bcm_phy_dt_ids[] = {
+	{ .compatible = "brcm,cygnus-usb-phy", },
+	{ }
+};
+
+static int bcm_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct bcm_phy_driver *phy_driver;
+	struct phy_provider *phy_provider;
+	struct device_node *child;
+	struct resource *res;
+	int error;
+	u32 reg_val;
+
+	phy_driver = devm_kzalloc(dev, sizeof(struct bcm_phy_driver),
+				  GFP_KERNEL);
+	if (!phy_driver)
+		return -ENOMEM;
+
+	spin_lock_init(&phy_driver->lock);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "missing memory resource usbphy_regs\n");
+		return -EINVAL;
+	}
+	phy_driver->usbphy_regs = devm_ioremap_nocache(dev, res->start,
+						resource_size(res));
+	if (!phy_driver->usbphy_regs) {
+		dev_err(dev, "Failed to remap usbphy_regs\n");
+		return -ENOMEM;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!res) {
+		dev_err(&pdev->dev, "missing memory resource ubs2h_idm_regs\n");
+		return -EINVAL;
+	}
+	phy_driver->usb2h_idm_regs = devm_ioremap_nocache(dev, res->start,
+						  resource_size(res));
+	if (!phy_driver->usb2h_idm_regs) {
+		dev_err(dev, "Failed to remap usb2h_idm_regs\n");
+		return -ENOMEM;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	if (!res) {
+		dev_err(&pdev->dev, "missing memory resource ubs2d_idm_regs\n");
+		return -EINVAL;
+	}
+	phy_driver->usb2d_idm_regs = devm_ioremap_nocache(dev, res->start,
+						   resource_size(res));
+	if (!phy_driver->usb2d_idm_regs) {
+		dev_err(dev, "Failed to remap usb2d_idm_regs\n");
+		return -ENOMEM;
+	}
+
+	dev_set_drvdata(dev, phy_driver);
+
+	phy_driver->idm_host_enabled = false;
+
+	/* Shut down all ports. They can be powered up as required */
+	reg_val = readl(phy_driver->usbphy_regs +
+			CRMU_USB_PHY_AON_CTRL_OFFSET);
+	reg_val &= ~(1 << CRMU_USBPHY_P0_AFE_CORERDY_VDDC);
+	reg_val &= ~(1 << CRMU_USBPHY_P0_RESETB);
+	reg_val &= ~(1 << CRMU_USBPHY_P1_AFE_CORERDY_VDDC);
+	reg_val &= ~(1 << CRMU_USBPHY_P1_RESETB);
+	reg_val &= ~(1 << CRMU_USBPHY_P2_AFE_CORERDY_VDDC);
+	reg_val &= ~(1 << CRMU_USBPHY_P2_RESETB);
+	writel(reg_val, phy_driver->usbphy_regs +
+		CRMU_USB_PHY_AON_CTRL_OFFSET);
+
+	for_each_available_child_of_node(dev->of_node, child) {
+		struct bcm_phy_instance *port;
+		u32 port_no;
+
+		if (of_property_read_u32(child, "reg", &port_no)) {
+			dev_err(dev, "missing reg property in node %s\n",
+				child->name);
+			return -EINVAL;
+		}
+
+		if (port_no >= ARRAY_SIZE(phy_driver->ports)) {
+			dev_err(dev, "invalid reg in node %s: %u\n",
+				child->name, port_no);
+			return -EINVAL;
+		}
+
+		port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
+		if (!port)
+			return -ENOMEM;
+
+		port->generic_phy = devm_phy_create(dev, child, &ops);
+		if (IS_ERR(port->generic_phy)) {
+			error = PTR_ERR(port->generic_phy);
+			dev_err(dev, "Failed to create phy %u: %d",
+				port_no, error);
+			return error;
+		}
+
+		port->port_no = port_no;
+		port->driver = phy_driver;
+		phy_set_drvdata(port->generic_phy, port);
+
+		phy_driver->ports[port_no] = port;
+	}
+
+	phy_provider = devm_of_phy_provider_register(dev, bcm_usb_phy_xlate);
+	if (IS_ERR(phy_provider)) {
+		error = PTR_ERR(phy_provider);
+		dev_err(dev, "Failed to register as phy provider: %d\n",
+			error);
+		return error;
+	}
+
+	dev_set_drvdata(pdev, phy_driver);
+
+	return 0;
+
+}
+
+MODULE_DEVICE_TABLE(of, bcm_phy_dt_ids);
+
+static struct platform_driver bcm_phy_driver = {
+	.probe = bcm_phy_probe,
+	.driver = {
+		.name = "bcm-cygnus-usbphy",
+		.of_match_table = of_match_ptr(bcm_phy_dt_ids),
+	},
+};
+module_platform_driver(bcm_phy_driver);
+
+MODULE_ALIAS("platform:bcm-cygnus-usbphy");
+MODULE_AUTHOR("Broadcom");
+MODULE_DESCRIPTION("Broadcom Cygnus USB PHY driver");
+MODULE_LICENSE("GPL v2");
-- 
2.3.4


^ permalink raw reply related

* Re: [PATCHv3 0/4] add devm_of_phy_get_by_index and update platform drivers
From: Hans de Goede @ 2015-04-23  7:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1429743853-10254-1-git-send-email-arun.ramamurthy@broadcom.com>

Hi,

On 23-04-15 01:04, Arun Ramamurthy wrote:
> This patch set adds a new API to get phy by index when multiple
> phys are present. This patch is based on discussion with Arnd Bergmann
> about dt bindings for multiple phys.
>
> History:
> v1:
>      - Removed null pointers on Dmitry's suggestion
>      - Improved documentation in commit messages
>      - Exported new phy api
> v2:
>      - EHCI and OHCI platform Kconfigs select Generic Phy
>        to fix build errors in certain configs.
> v3:
>      - Made GENERIC_PHY an invisible option so
>      that other configs can select it
>      - Added stubs for devm_of_phy_get_by_index
>      - Reformated code
>
> Arun Ramamurthy (4):
>    phy: phy-core: Make GENERIC_PHY an invisible option
>    phy: core: Add devm_of_phy_get_by_index to phy-core
>    usb: ehci-platform: Use devm_of_phy_get_by_index
>    usb: ohci-platform: Use devm_of_phy_get_by_index
>
>   Documentation/phy.txt                     |  7 +++-
>   drivers/ata/Kconfig                       |  1 -
>   drivers/media/platform/exynos4-is/Kconfig |  2 +-
>   drivers/phy/Kconfig                       |  4 +-
>   drivers/phy/phy-core.c                    | 32 ++++++++++++++
>   drivers/usb/host/Kconfig                  |  4 +-
>   drivers/usb/host/ehci-platform.c          | 69 +++++++++++--------------------
>   drivers/usb/host/ohci-platform.c          | 69 +++++++++++--------------------
>   drivers/video/fbdev/exynos/Kconfig        |  2 +-
>   include/linux/phy/phy.h                   |  8 ++++
>   10 files changed, 100 insertions(+), 98 deletions(-)
>

Patch set looks good to me:

Acked-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

^ permalink raw reply

* Re: [PATCH v3 05/17] video: fbdev: matrox: use arch_phys_wc_add() and ioremap_wc()
From: Julia Lawall @ 2015-04-23  8:20 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: plagnioj, tomi.valkeinen, linux-fbdev, luto, cocci,
	Luis R. Rodriguez, Rob Clark, Daniel Vetter, Geert Uytterhoeven,
	Julia Lawall, Mikulas Patocka, Suresh Siddha, Ingo Molnar,
	Thomas Gleixner, Juergen Gross, Dave Airlie, Antonino Daplas,
	linux-kernel
In-Reply-To: <1429647398-16983-6-git-send-email-mcgrof@do-not-panic.com>

> @ mtrr_found @
> expression index, base, size;
> @@
>
> -index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
> +index = arch_phys_wc_add(base, size);
>
> @ mtrr_rm depends on mtrr_found @
> expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
> @@
>
> -mtrr_del(index, base, size);
> +arch_phys_wc_del(index);
>
> @ mtrr_rm_zero_arg depends on mtrr_found @
> expression mtrr_found.index;
> @@
>
> -mtrr_del(index, 0, 0);
> +arch_phys_wc_del(index);
>
> @ mtrr_rm_fb_info depends on mtrr_found @
> struct fb_info *info;

Is this specific to the fb_info type?

julia


> expression mtrr_found.index;
> @@
>
> -mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
> +arch_phys_wc_del(index);
>
> @ ioremap_replace_nocache depends on mtrr_found @
> struct fb_info *info;
> expression base, size;
> @@
>
> -info->screen_base = ioremap_nocache(base, size);
> +info->screen_base = ioremap_wc(base, size);
>
> @ ioremap_replace_default depends on mtrr_found @
> struct fb_info *info;
> expression base, size;
> @@
>
> -info->screen_base = ioremap(base, size);
> +info->screen_base = ioremap_wc(base, size);
>
> Generated-by: Coccinelle SmPL
>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Julia Lawall <Julia.Lawall@lip6.fr>
> Cc: Mikulas Patocka <mpatocka@redhat.com>
> Cc: Suresh Siddha <sbsiddha@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Antonino Daplas <adaplas@gmail.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> ---
>  drivers/video/fbdev/matrox/matroxfb_base.c | 36 +++++++++++-------------------
>  drivers/video/fbdev/matrox/matroxfb_base.h | 27 +---------------------
>  2 files changed, 14 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/video/fbdev/matrox/matroxfb_base.c b/drivers/video/fbdev/matrox/matroxfb_base.c
> index 62539ca..2f70365 100644
> --- a/drivers/video/fbdev/matrox/matroxfb_base.c
> +++ b/drivers/video/fbdev/matrox/matroxfb_base.c
> @@ -370,12 +370,9 @@ static void matroxfb_remove(struct matrox_fb_info *minfo, int dummy)
>  	matroxfb_unregister_device(minfo);
>  	unregister_framebuffer(&minfo->fbcon);
>  	matroxfb_g450_shutdown(minfo);
> -#ifdef CONFIG_MTRR
> -	if (minfo->mtrr.vram_valid)
> -		mtrr_del(minfo->mtrr.vram, minfo->video.base, minfo->video.len);
> -#endif
> -	mga_iounmap(minfo->mmio.vbase);
> -	mga_iounmap(minfo->video.vbase);
> +	arch_phys_wc_del(minfo->wc_cookie);
> +	iounmap(minfo->mmio.vbase.vaddr);
> +	iounmap(minfo->video.vbase.vaddr);
>  	release_mem_region(minfo->video.base, minfo->video.len_maximum);
>  	release_mem_region(minfo->mmio.base, 16384);
>  	kfree(minfo);
> @@ -1256,9 +1253,7 @@ static int nobios;			/* "matroxfb:nobios" */
>  static int noinit = 1;			/* "matroxfb:init" */
>  static int inverse;			/* "matroxfb:inverse" */
>  static int sgram;			/* "matroxfb:sgram" */
> -#ifdef CONFIG_MTRR
>  static int mtrr = 1;			/* "matroxfb:nomtrr" */
> -#endif
>  static int grayscale;			/* "matroxfb:grayscale" */
>  static int dev = -1;			/* "matroxfb:dev:xxxxx" */
>  static unsigned int vesa = ~0;		/* "matroxfb:vesa:xxxxx" */
> @@ -1717,14 +1712,17 @@ static int initMatrox2(struct matrox_fb_info *minfo, struct board *b)
>  	if (mem && (mem < memsize))
>  		memsize = mem;
>  	err = -ENOMEM;
> -	if (mga_ioremap(ctrlptr_phys, 16384, MGA_IOREMAP_MMIO, &minfo->mmio.vbase)) {
> +
> +	minfo->mmio.vbase.vaddr = ioremap_nocache(ctrlptr_phys, 16384);
> +	if (!minfo->mmio.vbase.vaddr) {
>  		printk(KERN_ERR "matroxfb: cannot ioremap(%lX, 16384), matroxfb disabled\n", ctrlptr_phys);
>  		goto failVideoMR;
>  	}
>  	minfo->mmio.base = ctrlptr_phys;
>  	minfo->mmio.len = 16384;
>  	minfo->video.base = video_base_phys;
> -	if (mga_ioremap(video_base_phys, memsize, MGA_IOREMAP_FB, &minfo->video.vbase)) {
> +	minfo->video.vbase.vaddr = ioremap_wc(video_base_phys, memsize);
> +	if (!minfo->video.vbase.vaddr) {
>  		printk(KERN_ERR "matroxfb: cannot ioremap(%lX, %d), matroxfb disabled\n",
>  			video_base_phys, memsize);
>  		goto failCtrlIO;
> @@ -1772,13 +1770,9 @@ static int initMatrox2(struct matrox_fb_info *minfo, struct board *b)
>  	minfo->video.len_usable = minfo->video.len;
>  	if (minfo->video.len_usable > b->base->maxdisplayable)
>  		minfo->video.len_usable = b->base->maxdisplayable;
> -#ifdef CONFIG_MTRR
> -	if (mtrr) {
> -		minfo->mtrr.vram = mtrr_add(video_base_phys, minfo->video.len, MTRR_TYPE_WRCOMB, 1);
> -		minfo->mtrr.vram_valid = 1;
> -		printk(KERN_INFO "matroxfb: MTRR's turned on\n");
> -	}
> -#endif	/* CONFIG_MTRR */
> +	if (mtrr)
> +		minfo->wc_cookie = arch_phys_wc_add(video_base_phys,
> +						    minfo->video.len);
>
>  	if (!minfo->devflags.novga)
>  		request_region(0x3C0, 32, "matrox");
> @@ -1947,9 +1941,9 @@ static int initMatrox2(struct matrox_fb_info *minfo, struct board *b)
>  	return 0;
>  failVideoIO:;
>  	matroxfb_g450_shutdown(minfo);
> -	mga_iounmap(minfo->video.vbase);
> +	iounmap(minfo->video.vbase.vaddr);
>  failCtrlIO:;
> -	mga_iounmap(minfo->mmio.vbase);
> +	iounmap(minfo->mmio.vbase.vaddr);
>  failVideoMR:;
>  	release_mem_region(video_base_phys, minfo->video.len_maximum);
>  failCtrlMR:;
> @@ -2443,10 +2437,8 @@ static int __init matroxfb_setup(char *options) {
>  				nobios = !value;
>  			else if (!strcmp(this_opt, "init"))
>  				noinit = !value;
> -#ifdef CONFIG_MTRR
>  			else if (!strcmp(this_opt, "mtrr"))
>  				mtrr = value;
> -#endif
>  			else if (!strcmp(this_opt, "inv24"))
>  				inv24 = value;
>  			else if (!strcmp(this_opt, "cross4MB"))
> @@ -2515,10 +2507,8 @@ module_param(noinit, int, 0);
>  MODULE_PARM_DESC(noinit, "Disables W/SG/SD-RAM and bus interface initialization (0 or 1=do not initialize) (default=0)");
>  module_param(memtype, int, 0);
>  MODULE_PARM_DESC(memtype, "Memory type for G200/G400 (see Documentation/fb/matroxfb.txt for explanation) (default=3 for G200, 0 for G400)");
> -#ifdef CONFIG_MTRR
>  module_param(mtrr, int, 0);
>  MODULE_PARM_DESC(mtrr, "This speeds up video memory accesses (0=disabled or 1) (default=1)");
> -#endif
>  module_param(sgram, int, 0);
>  MODULE_PARM_DESC(sgram, "Indicates that G100/G200/G400 has SGRAM memory (0=SDRAM, 1=SGRAM) (default=0)");
>  module_param(inv24, int, 0);
> diff --git a/drivers/video/fbdev/matrox/matroxfb_base.h b/drivers/video/fbdev/matrox/matroxfb_base.h
> index 89a8a89a..09b02cd 100644
> --- a/drivers/video/fbdev/matrox/matroxfb_base.h
> +++ b/drivers/video/fbdev/matrox/matroxfb_base.h
> @@ -44,9 +44,6 @@
>
>  #include <asm/io.h>
>  #include <asm/unaligned.h>
> -#ifdef CONFIG_MTRR
> -#include <asm/mtrr.h>
> -#endif
>
>  #if defined(CONFIG_PPC_PMAC)
>  #include <asm/prom.h>
> @@ -187,23 +184,6 @@ static inline void __iomem* vaddr_va(vaddr_t va) {
>  	return va.vaddr;
>  }
>
> -#define MGA_IOREMAP_NORMAL	0
> -#define MGA_IOREMAP_NOCACHE	1
> -
> -#define MGA_IOREMAP_FB		MGA_IOREMAP_NOCACHE
> -#define MGA_IOREMAP_MMIO	MGA_IOREMAP_NOCACHE
> -static inline int mga_ioremap(unsigned long phys, unsigned long size, int flags, vaddr_t* virt) {
> -	if (flags & MGA_IOREMAP_NOCACHE)
> -		virt->vaddr = ioremap_nocache(phys, size);
> -	else
> -		virt->vaddr = ioremap(phys, size);
> -	return (virt->vaddr = NULL); /* 0, !0... 0, error_code in future */
> -}
> -
> -static inline void mga_iounmap(vaddr_t va) {
> -	iounmap(va.vaddr);
> -}
> -
>  struct my_timming {
>  	unsigned int pixclock;
>  	int mnp;
> @@ -449,12 +429,7 @@ struct matrox_fb_info {
>  		int		plnwt;
>  		int		srcorg;
>  			      } capable;
> -#ifdef CONFIG_MTRR
> -	struct {
> -		int		vram;
> -		int		vram_valid;
> -			      } mtrr;
> -#endif
> +	int			wc_cookie;
>  	struct {
>  		int		precise_width;
>  		int		mga_24bpp_fix;
> --
> 2.3.2.209.gd67f9d5.dirty
>
>

^ permalink raw reply

* [PATCH 1/2] staging: sm7xxfb: use framebuffer_alloc and release
From: Sudip Mukherjee @ 2015-04-23 13:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-fbdev, devel, linux-kernel, Sudip Mukherjee

use the standard framebuffer_alloc() and framebuffer_release() instead
of custom defined function. for making that change we had to change a
member of the private structure from a variable to pointer and had to
touch almost all places of the file. since fb was changed into a pointer
so all instance of "sfb->fb." has been changed into "sfb->fb->".
now we will get build warning about smtc_alloc_fb_info() and
smtc_free_fb_info() to be unused which will be removed in the next
patch of the series.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

tested on hardware.

 drivers/staging/sm7xxfb/sm7xxfb.c | 208 ++++++++++++++++++++------------------
 1 file changed, 110 insertions(+), 98 deletions(-)

diff --git a/drivers/staging/sm7xxfb/sm7xxfb.c b/drivers/staging/sm7xxfb/sm7xxfb.c
index 77f51a0..27f339f 100644
--- a/drivers/staging/sm7xxfb/sm7xxfb.c
+++ b/drivers/staging/sm7xxfb/sm7xxfb.c
@@ -39,7 +39,7 @@
 */
 struct smtcfb_info {
 	struct pci_dev *pdev;
-	struct fb_info fb;
+	struct fb_info *fb;
 	u16 chip_id;
 	u8  chip_rev_id;
 
@@ -249,19 +249,20 @@ static int smtc_setcolreg(unsigned regno, unsigned red, unsigned green,
 	if (regno > 255)
 		return 1;
 
-	switch (sfb->fb.fix.visual) {
+	switch (sfb->fb->fix.visual) {
 	case FB_VISUAL_DIRECTCOLOR:
 	case FB_VISUAL_TRUECOLOR:
 		/*
 		 * 16/32 bit true-colour, use pseudo-palette for 16 base color
 		 */
 		if (regno < 16) {
-			if (sfb->fb.var.bits_per_pixel = 16) {
-				u32 *pal = sfb->fb.pseudo_palette;
+			if (sfb->fb->var.bits_per_pixel = 16) {
+				u32 *pal = sfb->fb->pseudo_palette;
 
-				val = chan_to_field(red, &sfb->fb.var.red);
-				val |= chan_to_field(green, &sfb->fb.var.green);
-				val |= chan_to_field(blue, &sfb->fb.var.blue);
+				val = chan_to_field(red, &sfb->fb->var.red);
+				val |= chan_to_field(green,
+						     &sfb->fb->var.green);
+				val |= chan_to_field(blue, &sfb->fb->var.blue);
 #ifdef __BIG_ENDIAN
 				pal[regno]  				    ((red & 0xf800) >> 8) |
@@ -272,11 +273,12 @@ static int smtc_setcolreg(unsigned regno, unsigned red, unsigned green,
 				pal[regno] = val;
 #endif
 			} else {
-				u32 *pal = sfb->fb.pseudo_palette;
+				u32 *pal = sfb->fb->pseudo_palette;
 
-				val = chan_to_field(red, &sfb->fb.var.red);
-				val |= chan_to_field(green, &sfb->fb.var.green);
-				val |= chan_to_field(blue, &sfb->fb.var.blue);
+				val = chan_to_field(red, &sfb->fb->var.red);
+				val |= chan_to_field(green,
+						     &sfb->fb->var.green);
+				val |= chan_to_field(blue, &sfb->fb->var.blue);
 #ifdef __BIG_ENDIAN
 				val  				    (val & 0xff00ff00 >> 8) |
@@ -472,13 +474,13 @@ static void sm7xx_set_timing(struct smtcfb_info *sfb)
 	u32 m_nscreenstride;
 
 	dev_dbg(&sfb->pdev->dev,
-		"sfb->width=%d sfb->height=%d sfb->fb.var.bits_per_pixel=%d sfb->hz=%d\n",
-		sfb->width, sfb->height, sfb->fb.var.bits_per_pixel, sfb->hz);
+		"sfb->width=%d sfb->height=%d sfb->fb->var.bits_per_pixel=%d sfb->hz=%d\n",
+		sfb->width, sfb->height, sfb->fb->var.bits_per_pixel, sfb->hz);
 
 	for (j = 0; j < numvgamodes; j++) {
 		if (vgamode[j].mmsizex = sfb->width &&
 		    vgamode[j].mmsizey = sfb->height &&
-		    vgamode[j].bpp = sfb->fb.var.bits_per_pixel &&
+		    vgamode[j].bpp = sfb->fb->var.bits_per_pixel &&
 		    vgamode[j].hz = sfb->hz) {
 			dev_dbg(&sfb->pdev->dev,
 				"vgamode[j].mmsizex=%d vgamode[j].mmSizeY=%d vgamode[j].bpp=%d vgamode[j].hz=%d\n",
@@ -551,8 +553,8 @@ static void sm7xx_set_timing(struct smtcfb_info *sfb)
 
 	/* set data width */
 	m_nscreenstride -		(sfb->width * sfb->fb.var.bits_per_pixel) / 64;
-	switch (sfb->fb.var.bits_per_pixel) {
+		(sfb->width * sfb->fb->var.bits_per_pixel) / 64;
+	switch (sfb->fb->var.bits_per_pixel) {
 	case 8:
 		writel(0x0, sfb->vp_regs + 0x0);
 		break;
@@ -583,52 +585,52 @@ static void smtc_set_timing(struct smtcfb_info *sfb)
 
 static void smtcfb_setmode(struct smtcfb_info *sfb)
 {
-	switch (sfb->fb.var.bits_per_pixel) {
+	switch (sfb->fb->var.bits_per_pixel) {
 	case 32:
-		sfb->fb.fix.visual       = FB_VISUAL_TRUECOLOR;
-		sfb->fb.fix.line_length  = sfb->fb.var.xres * 4;
-		sfb->fb.var.red.length   = 8;
-		sfb->fb.var.green.length = 8;
-		sfb->fb.var.blue.length  = 8;
-		sfb->fb.var.red.offset   = 16;
-		sfb->fb.var.green.offset = 8;
-		sfb->fb.var.blue.offset  = 0;
+		sfb->fb->fix.visual       = FB_VISUAL_TRUECOLOR;
+		sfb->fb->fix.line_length  = sfb->fb->var.xres * 4;
+		sfb->fb->var.red.length   = 8;
+		sfb->fb->var.green.length = 8;
+		sfb->fb->var.blue.length  = 8;
+		sfb->fb->var.red.offset   = 16;
+		sfb->fb->var.green.offset = 8;
+		sfb->fb->var.blue.offset  = 0;
 		break;
 	case 24:
-		sfb->fb.fix.visual       = FB_VISUAL_TRUECOLOR;
-		sfb->fb.fix.line_length  = sfb->fb.var.xres * 3;
-		sfb->fb.var.red.length   = 8;
-		sfb->fb.var.green.length = 8;
-		sfb->fb.var.blue.length  = 8;
-		sfb->fb.var.red.offset   = 16;
-		sfb->fb.var.green.offset = 8;
-		sfb->fb.var.blue.offset  = 0;
+		sfb->fb->fix.visual       = FB_VISUAL_TRUECOLOR;
+		sfb->fb->fix.line_length  = sfb->fb->var.xres * 3;
+		sfb->fb->var.red.length   = 8;
+		sfb->fb->var.green.length = 8;
+		sfb->fb->var.blue.length  = 8;
+		sfb->fb->var.red.offset   = 16;
+		sfb->fb->var.green.offset = 8;
+		sfb->fb->var.blue.offset  = 0;
 		break;
 	case 8:
-		sfb->fb.fix.visual       = FB_VISUAL_PSEUDOCOLOR;
-		sfb->fb.fix.line_length  = sfb->fb.var.xres;
-		sfb->fb.var.red.length   = 3;
-		sfb->fb.var.green.length = 3;
-		sfb->fb.var.blue.length  = 2;
-		sfb->fb.var.red.offset   = 5;
-		sfb->fb.var.green.offset = 2;
-		sfb->fb.var.blue.offset  = 0;
+		sfb->fb->fix.visual       = FB_VISUAL_PSEUDOCOLOR;
+		sfb->fb->fix.line_length  = sfb->fb->var.xres;
+		sfb->fb->var.red.length   = 3;
+		sfb->fb->var.green.length = 3;
+		sfb->fb->var.blue.length  = 2;
+		sfb->fb->var.red.offset   = 5;
+		sfb->fb->var.green.offset = 2;
+		sfb->fb->var.blue.offset  = 0;
 		break;
 	case 16:
 	default:
-		sfb->fb.fix.visual       = FB_VISUAL_TRUECOLOR;
-		sfb->fb.fix.line_length  = sfb->fb.var.xres * 2;
-		sfb->fb.var.red.length   = 5;
-		sfb->fb.var.green.length = 6;
-		sfb->fb.var.blue.length  = 5;
-		sfb->fb.var.red.offset   = 11;
-		sfb->fb.var.green.offset = 5;
-		sfb->fb.var.blue.offset  = 0;
+		sfb->fb->fix.visual       = FB_VISUAL_TRUECOLOR;
+		sfb->fb->fix.line_length  = sfb->fb->var.xres * 2;
+		sfb->fb->var.red.length   = 5;
+		sfb->fb->var.green.length = 6;
+		sfb->fb->var.blue.length  = 5;
+		sfb->fb->var.red.offset   = 11;
+		sfb->fb->var.green.offset = 5;
+		sfb->fb->var.blue.offset  = 0;
 		break;
 	}
 
-	sfb->width  = sfb->fb.var.xres;
-	sfb->height = sfb->fb.var.yres;
+	sfb->width  = sfb->fb->var.xres;
+	sfb->height = sfb->fb->var.yres;
 	sfb->hz = 60;
 	smtc_set_timing(sfb);
 }
@@ -686,12 +688,12 @@ static struct smtcfb_info *smtc_alloc_fb_info(struct pci_dev *pdev)
 
 	sfb->pdev = pdev;
 
-	sfb->fb.flags          = FBINFO_FLAG_DEFAULT;
-	sfb->fb.fbops          = &smtcfb_ops;
-	sfb->fb.fix            = smtcfb_fix;
-	sfb->fb.var            = smtcfb_var;
-	sfb->fb.pseudo_palette = sfb->colreg;
-	sfb->fb.par            = sfb;
+	sfb->fb->flags          = FBINFO_FLAG_DEFAULT;
+	sfb->fb->fbops          = &smtcfb_ops;
+	sfb->fb->fix            = smtcfb_fix;
+	sfb->fb->var            = smtcfb_var;
+	sfb->fb->pseudo_palette = sfb->colreg;
+	sfb->fb->par            = sfb;
 
 	return sfb;
 }
@@ -721,20 +723,20 @@ static void smtc_unmap_mmio(struct smtcfb_info *sfb)
 static int smtc_map_smem(struct smtcfb_info *sfb,
 			 struct pci_dev *pdev, u_long smem_len)
 {
-	sfb->fb.fix.smem_start = pci_resource_start(pdev, 0);
+	sfb->fb->fix.smem_start = pci_resource_start(pdev, 0);
 
 #ifdef __BIG_ENDIAN
-	if (sfb->fb.var.bits_per_pixel = 32)
-		sfb->fb.fix.smem_start += 0x800000;
+	if (sfb->fb->var.bits_per_pixel = 32)
+		sfb->fb->fix.smem_start += 0x800000;
 #endif
 
-	sfb->fb.fix.smem_len = smem_len;
+	sfb->fb->fix.smem_len = smem_len;
 
-	sfb->fb.screen_base = sfb->lfb;
+	sfb->fb->screen_base = sfb->lfb;
 
-	if (!sfb->fb.screen_base) {
+	if (!sfb->fb->screen_base) {
 		dev_err(&pdev->dev,
-			"%s: unable to map screen memory\n", sfb->fb.fix.id);
+			"%s: unable to map screen memory\n", sfb->fb->fix.id);
 		return -ENOMEM;
 	}
 
@@ -747,9 +749,9 @@ static int smtc_map_smem(struct smtcfb_info *sfb,
  */
 static void smtc_unmap_smem(struct smtcfb_info *sfb)
 {
-	if (sfb && sfb->fb.screen_base) {
-		iounmap(sfb->fb.screen_base);
-		sfb->fb.screen_base = NULL;
+	if (sfb && sfb->fb->screen_base) {
+		iounmap(sfb->fb->screen_base);
+		sfb->fb->screen_base = NULL;
 	}
 }
 
@@ -766,6 +768,7 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *ent)
 {
 	struct smtcfb_info *sfb;
+	struct fb_info *info;
 	u_long smem_size = 0x00800000;	/* default 8MB */
 	int err;
 	unsigned long mmio_base;
@@ -784,14 +787,23 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
 
 	sprintf(smtcfb_fix.id, "sm%Xfb", ent->device);
 
-	sfb = smtc_alloc_fb_info(pdev);
-
-	if (!sfb) {
+	info = framebuffer_alloc(sizeof(*sfb), &pdev->dev);
+	if (!info) {
+		dev_err(&pdev->dev, "framebuffer_alloc failed\n");
 		err = -ENOMEM;
 		goto failed_free;
 	}
 
+	sfb = info->par;
+	sfb->fb = info;
 	sfb->chip_id = ent->device;
+	sfb->pdev = pdev;
+	info->flags = FBINFO_FLAG_DEFAULT;
+	info->fbops = &smtcfb_ops;
+	info->fix = smtcfb_fix;
+	info->var = smtcfb_var;
+	info->pseudo_palette = sfb->colreg;
+	info->par = sfb;
 
 	pci_set_drvdata(pdev, sfb);
 
@@ -799,19 +811,19 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
 
 	/* get mode parameter from smtc_scr_info */
 	if (smtc_scr_info.lfb_width != 0) {
-		sfb->fb.var.xres = smtc_scr_info.lfb_width;
-		sfb->fb.var.yres = smtc_scr_info.lfb_height;
-		sfb->fb.var.bits_per_pixel = smtc_scr_info.lfb_depth;
+		sfb->fb->var.xres = smtc_scr_info.lfb_width;
+		sfb->fb->var.yres = smtc_scr_info.lfb_height;
+		sfb->fb->var.bits_per_pixel = smtc_scr_info.lfb_depth;
 	} else {
 		/* default resolution 1024x600 16bit mode */
-		sfb->fb.var.xres = SCREEN_X_RES;
-		sfb->fb.var.yres = SCREEN_Y_RES;
-		sfb->fb.var.bits_per_pixel = SCREEN_BPP;
+		sfb->fb->var.xres = SCREEN_X_RES;
+		sfb->fb->var.yres = SCREEN_Y_RES;
+		sfb->fb->var.bits_per_pixel = SCREEN_BPP;
 	}
 
 #ifdef __BIG_ENDIAN
-	if (sfb->fb.var.bits_per_pixel = 24)
-		sfb->fb.var.bits_per_pixel = (smtc_scr_info.lfb_depth = 32);
+	if (sfb->fb->var.bits_per_pixel = 24)
+		sfb->fb->var.bits_per_pixel = (smtc_scr_info.lfb_depth = 32);
 #endif
 	/* Map address and memory detection */
 	mmio_base = pci_resource_start(pdev, 0);
@@ -820,8 +832,8 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
 	switch (sfb->chip_id) {
 	case 0x710:
 	case 0x712:
-		sfb->fb.fix.mmio_start = mmio_base + 0x00400000;
-		sfb->fb.fix.mmio_len = 0x00400000;
+		sfb->fb->fix.mmio_start = mmio_base + 0x00400000;
+		sfb->fb->fix.mmio_len = 0x00400000;
 		smem_size = SM712_VIDEOMEMORYSIZE;
 #ifdef __BIG_ENDIAN
 		sfb->lfb = ioremap(mmio_base, 0x00c00000);
@@ -833,7 +845,7 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
 		sfb->dp_regs = sfb->lfb + 0x00408000;
 		sfb->vp_regs = sfb->lfb + 0x0040c000;
 #ifdef __BIG_ENDIAN
-		if (sfb->fb.var.bits_per_pixel = 32) {
+		if (sfb->fb->var.bits_per_pixel = 32) {
 			sfb->lfb += 0x800000;
 			dev_info(&pdev->dev, "sfb->lfb=%p", sfb->lfb);
 		}
@@ -841,7 +853,7 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
 		if (!smtc_regbaseaddress) {
 			dev_err(&pdev->dev,
 				"%s: unable to map memory mapped IO!",
-				sfb->fb.fix.id);
+				sfb->fb->fix.id);
 			err = -ENOMEM;
 			goto failed_fb;
 		}
@@ -854,13 +866,13 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
 		smtc_seqw(0x17, 0x20);
 		/* enable word swap */
 #ifdef __BIG_ENDIAN
-		if (sfb->fb.var.bits_per_pixel = 32)
+		if (sfb->fb->var.bits_per_pixel = 32)
 			smtc_seqw(0x17, 0x30);
 #endif
 		break;
 	case 0x720:
-		sfb->fb.fix.mmio_start = mmio_base;
-		sfb->fb.fix.mmio_len = 0x00200000;
+		sfb->fb->fix.mmio_start = mmio_base;
+		sfb->fb->fix.mmio_len = 0x00200000;
 		smem_size = SM722_VIDEOMEMORYSIZE;
 		sfb->dp_regs = ioremap(mmio_base, 0x00a00000);
 		sfb->lfb = sfb->dp_regs + 0x00200000;
@@ -880,25 +892,25 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
 	}
 
 	/* can support 32 bpp */
-	if (15 = sfb->fb.var.bits_per_pixel)
-		sfb->fb.var.bits_per_pixel = 16;
+	if (15 = sfb->fb->var.bits_per_pixel)
+		sfb->fb->var.bits_per_pixel = 16;
 
-	sfb->fb.var.xres_virtual = sfb->fb.var.xres;
-	sfb->fb.var.yres_virtual = sfb->fb.var.yres;
+	sfb->fb->var.xres_virtual = sfb->fb->var.xres;
+	sfb->fb->var.yres_virtual = sfb->fb->var.yres;
 	err = smtc_map_smem(sfb, pdev, smem_size);
 	if (err)
 		goto failed;
 
 	smtcfb_setmode(sfb);
 
-	err = register_framebuffer(&sfb->fb);
+	err = register_framebuffer(info);
 	if (err < 0)
 		goto failed;
 
 	dev_info(&pdev->dev,
 		 "Silicon Motion SM%X Rev%X primary display mode %dx%d-%d Init Complete.",
-		 sfb->chip_id, sfb->chip_rev_id, sfb->fb.var.xres,
-		 sfb->fb.var.yres, sfb->fb.var.bits_per_pixel);
+		 sfb->chip_id, sfb->chip_rev_id, sfb->fb->var.xres,
+		 sfb->fb->var.yres, sfb->fb->var.bits_per_pixel);
 
 	return 0;
 
@@ -908,7 +920,7 @@ failed:
 	smtc_unmap_smem(sfb);
 	smtc_unmap_mmio(sfb);
 failed_fb:
-	smtc_free_fb_info(sfb);
+	framebuffer_release(info);
 
 failed_free:
 	pci_release_region(pdev, 0);
@@ -940,8 +952,8 @@ static void smtcfb_pci_remove(struct pci_dev *pdev)
 	sfb = pci_get_drvdata(pdev);
 	smtc_unmap_smem(sfb);
 	smtc_unmap_mmio(sfb);
-	unregister_framebuffer(&sfb->fb);
-	smtc_free_fb_info(sfb);
+	unregister_framebuffer(sfb->fb);
+	framebuffer_release(sfb->fb);
 	pci_release_region(pdev, 0);
 	pci_disable_device(pdev);
 }
@@ -961,7 +973,7 @@ static int smtcfb_pci_suspend(struct device *device)
 	smtc_seqw(0x69, (smtc_seqr(0x69) & 0xf7));
 
 	console_lock();
-	fb_set_suspend(&sfb->fb, 1);
+	fb_set_suspend(sfb->fb, 1);
 	console_unlock();
 
 	/* additionally turn off all function blocks including internal PLLs */
@@ -989,7 +1001,7 @@ static int smtcfb_pci_resume(struct device *device)
 		/* enable PCI burst */
 		smtc_seqw(0x17, 0x20);
 #ifdef __BIG_ENDIAN
-		if (sfb->fb.var.bits_per_pixel = 32)
+		if (sfb->fb->var.bits_per_pixel = 32)
 			smtc_seqw(0x17, 0x30);
 #endif
 		break;
@@ -1006,7 +1018,7 @@ static int smtcfb_pci_resume(struct device *device)
 	smtcfb_setmode(sfb);
 
 	console_lock();
-	fb_set_suspend(&sfb->fb, 0);
+	fb_set_suspend(sfb->fb, 0);
 	console_unlock();
 
 	return 0;
-- 
1.8.1.2


^ permalink raw reply related

* [PATCH 2/2] staging: sm7xxfb: remove unused functions
From: Sudip Mukherjee @ 2015-04-23 13:50 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-fbdev, devel, linux-kernel, Sudip Mukherjee
In-Reply-To: <1429796297-11750-1-git-send-email-sudipm.mukherjee@gmail.com>

removed the smtc_alloc_fb_info() and smtc_free_fb_info() functions which
were not used anymore.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/sm7xxfb/sm7xxfb.c | 32 --------------------------------
 1 file changed, 32 deletions(-)

diff --git a/drivers/staging/sm7xxfb/sm7xxfb.c b/drivers/staging/sm7xxfb/sm7xxfb.c
index 27f339f..5db26f1 100644
--- a/drivers/staging/sm7xxfb/sm7xxfb.c
+++ b/drivers/staging/sm7xxfb/sm7xxfb.c
@@ -675,38 +675,6 @@ static struct fb_ops smtcfb_ops = {
 };
 
 /*
- * alloc struct smtcfb_info and assign default values
- */
-static struct smtcfb_info *smtc_alloc_fb_info(struct pci_dev *pdev)
-{
-	struct smtcfb_info *sfb;
-
-	sfb = kzalloc(sizeof(*sfb), GFP_KERNEL);
-
-	if (!sfb)
-		return NULL;
-
-	sfb->pdev = pdev;
-
-	sfb->fb->flags          = FBINFO_FLAG_DEFAULT;
-	sfb->fb->fbops          = &smtcfb_ops;
-	sfb->fb->fix            = smtcfb_fix;
-	sfb->fb->var            = smtcfb_var;
-	sfb->fb->pseudo_palette = sfb->colreg;
-	sfb->fb->par            = sfb;
-
-	return sfb;
-}
-
-/*
- * free struct smtcfb_info
- */
-static void smtc_free_fb_info(struct smtcfb_info *sfb)
-{
-	kfree(sfb);
-}
-
-/*
  * Unmap in the memory mapped IO registers
  */
 
-- 
1.8.1.2


^ permalink raw reply related

* Re: [PATCHv3 0/4] add devm_of_phy_get_by_index and update platform drivers
From: Alan Stern @ 2015-04-23 14:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1429743853-10254-1-git-send-email-arun.ramamurthy@broadcom.com>

On Wed, 22 Apr 2015, Arun Ramamurthy wrote:

> This patch set adds a new API to get phy by index when multiple 
> phys are present. This patch is based on discussion with Arnd Bergmann
> about dt bindings for multiple phys.
> 
> History:
> v1:
>     - Removed null pointers on Dmitry's suggestion
>     - Improved documentation in commit messages
>     - Exported new phy api
> v2:
>     - EHCI and OHCI platform Kconfigs select Generic Phy
>       to fix build errors in certain configs.
> v3:
>     - Made GENERIC_PHY an invisible option so 
>     that other configs can select it
>     - Added stubs for devm_of_phy_get_by_index
>     - Reformated code
> 
> Arun Ramamurthy (4):
>   phy: phy-core: Make GENERIC_PHY an invisible option
>   phy: core: Add devm_of_phy_get_by_index to phy-core
>   usb: ehci-platform: Use devm_of_phy_get_by_index
>   usb: ohci-platform: Use devm_of_phy_get_by_index

For patches 3 and 4:

Acked-by: Alan Stern <stern@rowland.harvard.edu>


^ permalink raw reply

* Re: [PATCH v3 05/17] video: fbdev: matrox: use arch_phys_wc_add() and ioremap_wc()
From: Luis R. Rodriguez @ 2015-04-23 16:46 UTC (permalink / raw)
  To: Julia Lawall, plagnioj, tomi.valkeinen, linux-fbdev, Rob Clark,
	Daniel Vetter, Dave Airlie
  Cc: Luis R. Rodriguez, luto, cocci, Geert Uytterhoeven,
	Mikulas Patocka, Suresh Siddha, Ingo Molnar, Thomas Gleixner,
	Juergen Gross, Antonino Daplas, linux-kernel
In-Reply-To: <alpine.DEB.2.10.1504231019190.3260@hadrien>

On Thu, Apr 23, 2015 at 10:20:10AM +0200, Julia Lawall wrote:
> > @ mtrr_found @
> > expression index, base, size;
> > @@
> >
> > -index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
> > +index = arch_phys_wc_add(base, size);
> >
> > @ mtrr_rm depends on mtrr_found @
> > expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
> > @@
> >
> > -mtrr_del(index, base, size);
> > +arch_phys_wc_del(index);
> >
> > @ mtrr_rm_zero_arg depends on mtrr_found @
> > expression mtrr_found.index;
> > @@
> >
> > -mtrr_del(index, 0, 0);
> > +arch_phys_wc_del(index);
> >
> > @ mtrr_rm_fb_info depends on mtrr_found @
> > struct fb_info *info;
> 
> Is this specific to the fb_info type?
> 

Glad you asked. Technically all framebuffer drivers will have
the fb_info and they *should* fill this properly with the base
and length. More on this below though.

> 
> > expression mtrr_found.index;
> > @@
> >
> > -mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
> > +arch_phys_wc_del(index);

Note: when I mention mtrr_add() I mean arch_phys_wc_add() as that
is what we have been changing the drivers to over the years and in
this series as well. Likewise for mtrr_del() there is arch_phys_wc_del(),
but since you might be revieiwng the code with mtrr_add() figured I'd
mention the current state of affairs prior to this set of series'
changes.

*Ideally* when the mtrr_add() or mtrr_del() call is made on framebuffer drivers
we'd be able to use info->fix.smem_start, info->fix.smem_len for those call's
purposes, where info is struct fb_info *info. I however did not find that to be
the case for all framebuffer device drivers, but it does not mean its not
possible. Even if its not possible for all framebuffer device drivers quite a
few do fall under this category and as such once all were vetted in grammar
form (maybe another cleanup on top of this series) I figured we could have
register_framebuffer() do the mtrr_add() call for drivers that need it by
simply setting a flag on a struct passed, likewise if such flag is set we'd
have the framebuffer core do the mtrr_del() later upon deregistration. I
considered doing this as another series but ran out of steam.

The only thing with this also though is we have DRM drivers and they have the DRM
core doing the mtrr stuff for them but I failed to complete the review there
and if we can just unify things for all framebuffer / DRM drivers I think that'd
be best, but it was also not clear if we want or if this is worth doing. Since
mtrr stuff is low hanging fruit and its all legacy stuff I didn't
bother to pursue more -- but in terms of grammar and cleanup there sure is
quite a bit of room left for love here. For a unified mtrr set of calls for
both framebuffer / DRM drivers it may be good to get guidance from fbdev / DRM
developers.

I hinted towards this on my original cover letter on the first version of
this series, see section d):

http://article.gmane.org/gmane.linux.kernel/1913979

BTW what I mean by "option" on that d) section of that cover letter I
mean all the driver string options passed to the driver, see matroxfb_setup()
for example, there is an option for mtrr.

  Luis

^ permalink raw reply

* [PATCH] staging: sm750fb: cleanup white space and indent
From: Charles Rose @ 2015-04-23 21:04 UTC (permalink / raw)
  To: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, devel,
	linux-kernel
  Cc: charles.rose.linux

This patch fixes "space prohibited" errors reported by checkpatch.pl
and related indentation. The module builds without error.

Signed-off-by: Charles Rose <charles.rose.linux@gmail.com>
---
 drivers/staging/sm750fb/ddk750_chip.c  | 12 ++++++------
 drivers/staging/sm750fb/ddk750_mode.c  | 26 +++++++++++++++++---------
 drivers/staging/sm750fb/ddk750_power.c |  8 ++++----
 drivers/staging/sm750fb/sm750_accel.c  |  2 +-
 drivers/staging/sm750fb/sm750_help.h   |  2 +-
 5 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_chip.c b/drivers/staging/sm750fb/ddk750_chip.c
index 7b28328..60ae39a 100644
--- a/drivers/staging/sm750fb/ddk750_chip.c
+++ b/drivers/staging/sm750fb/ddk750_chip.c
@@ -268,7 +268,7 @@ int ddk750_initHw(initchip_param_t *pInitParam)
 #endif
 
 
-	if (pInitParam->powerMode != 0 )
+	if (pInitParam->powerMode != 0)
 		pInitParam->powerMode = 0;
 	setPowerMode(pInitParam->powerMode);
 
@@ -285,7 +285,7 @@ int ddk750_initHw(initchip_param_t *pInitParam)
 		ulReg = FIELD_SET(ulReg, VGA_CONFIGURATION, MODE, GRAPHIC);
 		POKE32(VGA_CONFIGURATION, ulReg);
 	} else {
-#if defined(__i386__) || defined( __x86_64__)
+#if defined(__i386__) || defined(__x86_64__)
 		/* set graphic mode via IO method */
 		outb_p(0x88, 0x3d4);
 		outb_p(0x06, 0x3d5);
@@ -382,7 +382,7 @@ int ddk750_initHw(initchip_param_t *pInitParam)
 
 unsigned int absDiff(unsigned int a, unsigned int b)
 {
-	if ( a > b )
+	if (a > b)
 		return(a - b);
 	else
 		return(b - a);
@@ -606,9 +606,9 @@ unsigned int formatPllReg(pll_value_t *pPLL)
        On returning a 32 bit number, the value can be applied to any PLL in the calling function.
     */
 	ulPllReg -	FIELD_SET(  0, PANEL_PLL_CTRL, BYPASS, OFF)
-	| FIELD_SET(  0, PANEL_PLL_CTRL, POWER,  ON)
-	| FIELD_SET(  0, PANEL_PLL_CTRL, INPUT,  OSC)
+	FIELD_SET(0, PANEL_PLL_CTRL, BYPASS, OFF)
+	| FIELD_SET(0, PANEL_PLL_CTRL, POWER,  ON)
+	| FIELD_SET(0, PANEL_PLL_CTRL, INPUT,  OSC)
 #ifndef VALIDATION_CHIP
 	| FIELD_VALUE(0, PANEL_PLL_CTRL, POD,    pPLL->POD)
 #endif
diff --git a/drivers/staging/sm750fb/ddk750_mode.c b/drivers/staging/sm750fb/ddk750_mode.c
index 2e418fb..a7e2f9a 100644
--- a/drivers/staging/sm750fb/ddk750_mode.c
+++ b/drivers/staging/sm750fb/ddk750_mode.c
@@ -43,22 +43,30 @@ static unsigned long displayControlAdjust_SM750LE(mode_parameter_t *pModeParam,
 
 	/* Set bit 29:27 of display control register for the right clock */
 	/* Note that SM750LE only need to supported 7 resoluitons. */
-	if ( x = 800 && y = 600 )
-    	dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CLK, PLL41);
+	if (x = 800 && y = 600)
+		dispControl = FIELD_SET(dispControl,
+				CRT_DISPLAY_CTRL, CLK, PLL41);
 	else if (x = 1024 && y = 768)
-    	dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CLK, PLL65);
+		dispControl = FIELD_SET(dispControl,
+				CRT_DISPLAY_CTRL, CLK, PLL65);
 	else if (x = 1152 && y = 864)
-    	dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CLK, PLL80);
+		dispControl = FIELD_SET(dispControl,
+				CRT_DISPLAY_CTRL, CLK, PLL80);
 	else if (x = 1280 && y = 768)
-    	dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CLK, PLL80);
+		dispControl = FIELD_SET(dispControl,
+				CRT_DISPLAY_CTRL, CLK, PLL80);
 	else if (x = 1280 && y = 720)
-    	dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CLK, PLL74);
+		dispControl = FIELD_SET(dispControl,
+				CRT_DISPLAY_CTRL, CLK, PLL74);
 	else if (x = 1280 && y = 960)
-    	dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CLK, PLL108);
+		dispControl = FIELD_SET(dispControl,
+				CRT_DISPLAY_CTRL, CLK, PLL108);
 	else if (x = 1280 && y = 1024)
-    	dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CLK, PLL108);
+		dispControl = FIELD_SET(dispControl,
+				CRT_DISPLAY_CTRL, CLK, PLL108);
 	else /* default to VGA clock */
-    	dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CLK, PLL25);
+		dispControl = FIELD_SET(dispControl,
+				CRT_DISPLAY_CTRL, CLK, PLL25);
 
 	/* Set bit 25:24 of display controller */
     dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CRTSELECT, CRT);
diff --git a/drivers/staging/sm750fb/ddk750_power.c b/drivers/staging/sm750fb/ddk750_power.c
index cbb9767..0e3c028 100644
--- a/drivers/staging/sm750fb/ddk750_power.c
+++ b/drivers/staging/sm750fb/ddk750_power.c
@@ -59,17 +59,17 @@ void setPowerMode(unsigned int powerMode)
     {
         control_value  #ifdef VALIDATION_CHIP
-            FIELD_SET(  control_value, POWER_MODE_CTRL, 336CLK, OFF) |
+		FIELD_SET(control_value, POWER_MODE_CTRL, 336CLK, OFF) |
 #endif
-            FIELD_SET(  control_value, POWER_MODE_CTRL, OSC_INPUT,  OFF);
+		FIELD_SET(control_value, POWER_MODE_CTRL, OSC_INPUT, OFF);
     }
     else
     {
         control_value  #ifdef VALIDATION_CHIP
-            FIELD_SET(  control_value, POWER_MODE_CTRL, 336CLK, ON) |
+		FIELD_SET(control_value, POWER_MODE_CTRL, 336CLK, ON) |
 #endif
-            FIELD_SET(  control_value, POWER_MODE_CTRL, OSC_INPUT,  ON);
+		FIELD_SET(control_value, POWER_MODE_CTRL, OSC_INPUT, ON);
     }
 
     /* Program new power mode. */
diff --git a/drivers/staging/sm750fb/sm750_accel.c b/drivers/staging/sm750fb/sm750_accel.c
index c5a3726..e308646 100644
--- a/drivers/staging/sm750fb/sm750_accel.c
+++ b/drivers/staging/sm750fb/sm750_accel.c
@@ -364,7 +364,7 @@ int hw_imageblit(struct lynx_accel *accel,
        Note that input pitch is BYTE value, but the 2D Pitch register uses
        pixel values. Need Byte to pixel convertion.
     */
-	if(bytePerPixel = 3 ){
+	if (bytePerPixel = 3) {
 		dx *= 3;
 		width *= 3;
 		startBit *= 3;
diff --git a/drivers/staging/sm750fb/sm750_help.h b/drivers/staging/sm750fb/sm750_help.h
index e0128d2..cdac9e2 100644
--- a/drivers/staging/sm750fb/sm750_help.h
+++ b/drivers/staging/sm750fb/sm750_help.h
@@ -57,7 +57,7 @@
 
 #define FIELD_CLEAR(reg, field) \
 ( \
-    ~ _F_MASK(reg ## _ ## field) \
+	~_F_MASK(reg ## _ ## field) \
 )
 
 
-- 
2.1.0


^ permalink raw reply related

* Re: [PATCH] staging: sm750fb: cleanup white space and indent
From: Sudip Mukherjee @ 2015-04-24  7:14 UTC (permalink / raw)
  To: Charles Rose; +Cc: teddy.wang, gregkh, linux-fbdev, devel, linux-kernel
In-Reply-To: <1429823048-9094-1-git-send-email-charles.rose.linux@gmail.com>

On Thu, Apr 23, 2015 at 05:04:08PM -0400, Charles Rose wrote:
> This patch fixes "space prohibited" errors reported by checkpatch.pl
> and related indentation. The module builds without error.
> 
> Signed-off-by: Charles Rose <charles.rose.linux@gmail.com>
<snip>
> -	if ( x = 800 && y = 600 )
> -    	dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CLK, PLL41);
> +	if (x = 800 && y = 600)
> +		dispControl = FIELD_SET(dispControl,
> +				CRT_DISPLAY_CTRL, CLK, PLL41);

Alignment should match open parenthesis

regards
sudip

^ permalink raw reply

* Re: what's the difference between smem_start and mmio_start?
From: Sudip Mukherjee @ 2015-04-24  7:42 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <CADvLq87aq7m-xuf6XbOq7rDDjLORwiW-5O0qd5tfm+_H_xd05w@mail.gmail.com>

On Sat, Apr 18, 2015 at 03:46:48PM +0800, z f wrote:
> what's the difference between smem_start member and mmio_start member
> in struct fb_fix_screeninfo?

let me try, but I am not sure that I am fully correct.
smem_start gives you the starting address where your framebuffer memory
starts. mmio_start will give you the memory address from where you can
access memory mapped io ports.
I dont think that all the hardware will follow the same location rules.

like, if you consider the driver that I have in staging/sm7xxfb,
the memory map is like:

________________________________  0MB
|				|	
|  display memory		|
|				|
|				|
|_______________________________| 4MB
|				|
|_______________________________| 5MB
|				|
| memory mapped io port  	|
|				|
|_______________________________|

so here I have:

smem_start = pci_resource_start (pdev, 0);
mmio_start = smem_start + 4MB

but if you see the radeon driver you will see thay have
smem_start = pci_resource_start (pdev, 0);
and
mmio_start = pci_resource_start (pdev, 2);

regards
sudip

^ permalink raw reply

* Re: [PATCH] staging: sm750fb: cleanup white space and indent
From: Greg KH @ 2015-04-24  8:22 UTC (permalink / raw)
  To: Charles Rose
  Cc: sudipm.mukherjee, teddy.wang, linux-fbdev, devel, linux-kernel
In-Reply-To: <1429823048-9094-1-git-send-email-charles.rose.linux@gmail.com>

On Thu, Apr 23, 2015 at 05:04:08PM -0400, Charles Rose wrote:
> This patch fixes "space prohibited" errors reported by checkpatch.pl
> and related indentation. The module builds without error.

That's two different things, can you break this up into one "logical"
patch per thing you are changing here?

thanks,

greg k-h

^ permalink raw reply

* Re: simple framebuffer slower by factor of 20, on socfpga (arm) platform
From: Pavel Machek @ 2015-04-24 13:29 UTC (permalink / raw)
  To: Archit Taneja
  Cc: Tomi Valkeinen, Geert Uytterhoeven, Marek Vasut, kernel list,
	Dinh Nguyen, Jean-Christophe PLAGNIOL-VILLARD, Grant Likely,
	Rob Herring, Jingoo Han, Rob Clark, Linux Fbdev development list,
	devicetree@vger.kernel.org, shc_work, linux, hsweeten
In-Reply-To: <55277650.8070607@codeaurora.org>

Hi!

On Fri 2015-04-10 12:35:52, Archit Taneja wrote:
> >That said, if the fb is in RAM, and is only written by the CPU, I think
> >a normal memcpy() for fb_memcpy_fromfb() should be fine...
> 
> I didn't test for performance regressions when I posted this patch.
> 
> A look at _memcpy_fromio in arch/arm/kernel/io.c shows that readb() is used
> all the time, even when the source and destination addresses are aligned for
> larger reads to be possible. Other archs seem to use readl() or readq() when
> they can. Maybe that makes memcpy_fromio slower than the implementation of
> memcpy on arm?

Ok, can you prepare a patch for me to try? Or should we just revert
the original commit?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: simple framebuffer slower by factor of 20, on socfpga (arm) platform
From: Pavel Machek @ 2015-04-24 13:31 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Tomi Valkeinen, Geert Uytterhoeven, Marek Vasut, kernel list,
	Dinh Nguyen, Jean-Christophe PLAGNIOL-VILLARD, Grant Likely,
	Rob Herring, Jingoo Han, Rob Clark, Linux Fbdev development list,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	shc_work-JGs/UdohzUI, linux-lFZ/pmaqli7XmaaqVzeoHQ,
	hsweeten-3FF4nKcrg1dE2c76skzGb0EOCMrvLtNR, Archit Taneja
In-Reply-To: <2137270.OOdtDiT2H4@wuerfel>


> > The difference is probably caused by memcpy() vs memcpy_fromio(). The
> > comment above memcpy_fromio() says "This needs to be optimized". I think
> > generally speaking memcpy_fromio() is correct for a framebuffer.
> > 
> > That said, if the fb is in RAM, and is only written by the CPU, I think
> > a normal memcpy() for fb_memcpy_fromfb() should be fine...
> 
> Could memcpy() cause alignment traps here if the fb pointer is unaligned
> and uncached?

Original commit did not comment on any failure, so I expect that is
not a problem here...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: simple framebuffer slower by factor of 20, on socfpga (arm) platform
From: Tomi Valkeinen @ 2015-04-24 13:40 UTC (permalink / raw)
  To: Pavel Machek, Archit Taneja
  Cc: Geert Uytterhoeven, Marek Vasut, kernel list, Dinh Nguyen,
	Jean-Christophe PLAGNIOL-VILLARD, Grant Likely, Rob Herring,
	Jingoo Han, Rob Clark, Linux Fbdev development list,
	devicetree@vger.kernel.org, shc_work, linux, hsweeten
In-Reply-To: <20150424132923.GA11729@amd>

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

On 24/04/15 16:29, Pavel Machek wrote:
> Hi!
> 
> On Fri 2015-04-10 12:35:52, Archit Taneja wrote:
>>> That said, if the fb is in RAM, and is only written by the CPU, I think
>>> a normal memcpy() for fb_memcpy_fromfb() should be fine...
>>
>> I didn't test for performance regressions when I posted this patch.
>>
>> A look at _memcpy_fromio in arch/arm/kernel/io.c shows that readb() is used
>> all the time, even when the source and destination addresses are aligned for
>> larger reads to be possible. Other archs seem to use readl() or readq() when
>> they can. Maybe that makes memcpy_fromio slower than the implementation of
>> memcpy on arm?
> 
> Ok, can you prepare a patch for me to try? Or should we just revert
> the original commit?

The old way worked fine, afaik, so maybe we can revert. But still, isn't
it more correct to use memcpy_fromio? It's (possibly) io memory we have
here.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: simple framebuffer slower by factor of 20, on socfpga (arm) platform
From: Geert Uytterhoeven @ 2015-04-24 13:46 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Pavel Machek, Archit Taneja, Marek Vasut, kernel list,
	Dinh Nguyen, Jean-Christophe PLAGNIOL-VILLARD, Grant Likely,
	Rob Herring, Jingoo Han, Rob Clark, Linux Fbdev development list,
	devicetree@vger.kernel.org, Alexander Shiyan, Russell King,
	H Hartley Sweeten
In-Reply-To: <553A47D3.2070107@ti.com>

On Fri, Apr 24, 2015 at 3:40 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 24/04/15 16:29, Pavel Machek wrote:
>> On Fri 2015-04-10 12:35:52, Archit Taneja wrote:
>>>> That said, if the fb is in RAM, and is only written by the CPU, I think
>>>> a normal memcpy() for fb_memcpy_fromfb() should be fine...
>>>
>>> I didn't test for performance regressions when I posted this patch.
>>>
>>> A look at _memcpy_fromio in arch/arm/kernel/io.c shows that readb() is used
>>> all the time, even when the source and destination addresses are aligned for
>>> larger reads to be possible. Other archs seem to use readl() or readq() when
>>> they can. Maybe that makes memcpy_fromio slower than the implementation of
>>> memcpy on arm?
>>
>> Ok, can you prepare a patch for me to try? Or should we just revert
>> the original commit?
>
> The old way worked fine, afaik, so maybe we can revert. But still, isn't
> it more correct to use memcpy_fromio? It's (possibly) io memory we have
> here.

Yes it is.

So please optimize ARM's _memcpy_fromio(), _memcpy_toio(), and _memset_io().
That will benefit other drivers on ARM, too.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH v2 1/2] staging: sm750fb: cleanup white space
From: Charles Rose @ 2015-04-24 15:00 UTC (permalink / raw)
  To: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, devel,
	linux-kernel
  Cc: charles.rose.linux

This patch fixes "space prohibited" errors reported by checkpatch.pl

Signed-off-by: Charles Rose <charles.rose.linux@gmail.com>
---
 drivers/staging/sm750fb/ddk750_chip.c  | 12 ++++++------
 drivers/staging/sm750fb/ddk750_mode.c  |  2 +-
 drivers/staging/sm750fb/ddk750_power.c |  8 ++++----
 drivers/staging/sm750fb/sm750_accel.c  |  2 +-
 drivers/staging/sm750fb/sm750_help.h   |  2 +-
 5 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_chip.c b/drivers/staging/sm750fb/ddk750_chip.c
index 7b28328..60ae39a 100644
--- a/drivers/staging/sm750fb/ddk750_chip.c
+++ b/drivers/staging/sm750fb/ddk750_chip.c
@@ -268,7 +268,7 @@ int ddk750_initHw(initchip_param_t *pInitParam)
 #endif
 
 
-	if (pInitParam->powerMode != 0 )
+	if (pInitParam->powerMode != 0)
 		pInitParam->powerMode = 0;
 	setPowerMode(pInitParam->powerMode);
 
@@ -285,7 +285,7 @@ int ddk750_initHw(initchip_param_t *pInitParam)
 		ulReg = FIELD_SET(ulReg, VGA_CONFIGURATION, MODE, GRAPHIC);
 		POKE32(VGA_CONFIGURATION, ulReg);
 	} else {
-#if defined(__i386__) || defined( __x86_64__)
+#if defined(__i386__) || defined(__x86_64__)
 		/* set graphic mode via IO method */
 		outb_p(0x88, 0x3d4);
 		outb_p(0x06, 0x3d5);
@@ -382,7 +382,7 @@ int ddk750_initHw(initchip_param_t *pInitParam)
 
 unsigned int absDiff(unsigned int a, unsigned int b)
 {
-	if ( a > b )
+	if (a > b)
 		return(a - b);
 	else
 		return(b - a);
@@ -606,9 +606,9 @@ unsigned int formatPllReg(pll_value_t *pPLL)
        On returning a 32 bit number, the value can be applied to any PLL in the calling function.
     */
 	ulPllReg -	FIELD_SET(  0, PANEL_PLL_CTRL, BYPASS, OFF)
-	| FIELD_SET(  0, PANEL_PLL_CTRL, POWER,  ON)
-	| FIELD_SET(  0, PANEL_PLL_CTRL, INPUT,  OSC)
+	FIELD_SET(0, PANEL_PLL_CTRL, BYPASS, OFF)
+	| FIELD_SET(0, PANEL_PLL_CTRL, POWER,  ON)
+	| FIELD_SET(0, PANEL_PLL_CTRL, INPUT,  OSC)
 #ifndef VALIDATION_CHIP
 	| FIELD_VALUE(0, PANEL_PLL_CTRL, POD,    pPLL->POD)
 #endif
diff --git a/drivers/staging/sm750fb/ddk750_mode.c b/drivers/staging/sm750fb/ddk750_mode.c
index 2e418fb..8e7b9d4 100644
--- a/drivers/staging/sm750fb/ddk750_mode.c
+++ b/drivers/staging/sm750fb/ddk750_mode.c
@@ -43,7 +43,7 @@ static unsigned long displayControlAdjust_SM750LE(mode_parameter_t *pModeParam,
 
 	/* Set bit 29:27 of display control register for the right clock */
 	/* Note that SM750LE only need to supported 7 resoluitons. */
-	if ( x = 800 && y = 600 )
+	if (x = 800 && y = 600)
     	dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CLK, PLL41);
 	else if (x = 1024 && y = 768)
     	dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CLK, PLL65);
diff --git a/drivers/staging/sm750fb/ddk750_power.c b/drivers/staging/sm750fb/ddk750_power.c
index cbb9767..0e3c028 100644
--- a/drivers/staging/sm750fb/ddk750_power.c
+++ b/drivers/staging/sm750fb/ddk750_power.c
@@ -59,17 +59,17 @@ void setPowerMode(unsigned int powerMode)
     {
         control_value  #ifdef VALIDATION_CHIP
-            FIELD_SET(  control_value, POWER_MODE_CTRL, 336CLK, OFF) |
+		FIELD_SET(control_value, POWER_MODE_CTRL, 336CLK, OFF) |
 #endif
-            FIELD_SET(  control_value, POWER_MODE_CTRL, OSC_INPUT,  OFF);
+		FIELD_SET(control_value, POWER_MODE_CTRL, OSC_INPUT, OFF);
     }
     else
     {
         control_value  #ifdef VALIDATION_CHIP
-            FIELD_SET(  control_value, POWER_MODE_CTRL, 336CLK, ON) |
+		FIELD_SET(control_value, POWER_MODE_CTRL, 336CLK, ON) |
 #endif
-            FIELD_SET(  control_value, POWER_MODE_CTRL, OSC_INPUT,  ON);
+		FIELD_SET(control_value, POWER_MODE_CTRL, OSC_INPUT, ON);
     }
 
     /* Program new power mode. */
diff --git a/drivers/staging/sm750fb/sm750_accel.c b/drivers/staging/sm750fb/sm750_accel.c
index c5a3726..e308646 100644
--- a/drivers/staging/sm750fb/sm750_accel.c
+++ b/drivers/staging/sm750fb/sm750_accel.c
@@ -364,7 +364,7 @@ int hw_imageblit(struct lynx_accel *accel,
        Note that input pitch is BYTE value, but the 2D Pitch register uses
        pixel values. Need Byte to pixel convertion.
     */
-	if(bytePerPixel = 3 ){
+	if (bytePerPixel = 3) {
 		dx *= 3;
 		width *= 3;
 		startBit *= 3;
diff --git a/drivers/staging/sm750fb/sm750_help.h b/drivers/staging/sm750fb/sm750_help.h
index e0128d2..cdac9e2 100644
--- a/drivers/staging/sm750fb/sm750_help.h
+++ b/drivers/staging/sm750fb/sm750_help.h
@@ -57,7 +57,7 @@
 
 #define FIELD_CLEAR(reg, field) \
 ( \
-    ~ _F_MASK(reg ## _ ## field) \
+	~_F_MASK(reg ## _ ## field) \
 )
 
 
-- 
2.1.0


^ permalink raw reply related

* [PATCH v2 2/2] staging: sm750fb: cleanup indentation
From: Charles Rose @ 2015-04-24 15:10 UTC (permalink / raw)
  To: sudipm.mukherjee, teddy.wang, gregkh, linux-fbdev, devel,
	linux-kernel
  Cc: charles.rose.linux
In-Reply-To: <1429887612-15285-1-git-send-email-charles.rose.linux@gmail.com>

This patch fixes indentation errors/warnings reported by checkpatch.pl.

Signed-off-by: Charles Rose <charles.rose.linux@gmail.com>
---
 drivers/staging/sm750fb/ddk750_mode.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_mode.c b/drivers/staging/sm750fb/ddk750_mode.c
index 8e7b9d4..eac5712 100644
--- a/drivers/staging/sm750fb/ddk750_mode.c
+++ b/drivers/staging/sm750fb/ddk750_mode.c
@@ -44,21 +44,29 @@ static unsigned long displayControlAdjust_SM750LE(mode_parameter_t *pModeParam,
 	/* Set bit 29:27 of display control register for the right clock */
 	/* Note that SM750LE only need to supported 7 resoluitons. */
 	if (x = 800 && y = 600)
-    	dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CLK, PLL41);
+		dispControl = FIELD_SET(dispControl,
+					CRT_DISPLAY_CTRL, CLK, PLL41);
 	else if (x = 1024 && y = 768)
-    	dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CLK, PLL65);
+		dispControl = FIELD_SET(dispControl,
+					CRT_DISPLAY_CTRL, CLK, PLL65);
 	else if (x = 1152 && y = 864)
-    	dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CLK, PLL80);
+		dispControl = FIELD_SET(dispControl,
+					CRT_DISPLAY_CTRL, CLK, PLL80);
 	else if (x = 1280 && y = 768)
-    	dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CLK, PLL80);
+		dispControl = FIELD_SET(dispControl,
+					CRT_DISPLAY_CTRL, CLK, PLL80);
 	else if (x = 1280 && y = 720)
-    	dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CLK, PLL74);
+		dispControl = FIELD_SET(dispControl,
+					CRT_DISPLAY_CTRL, CLK, PLL74);
 	else if (x = 1280 && y = 960)
-    	dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CLK, PLL108);
+		dispControl = FIELD_SET(dispControl,
+					CRT_DISPLAY_CTRL, CLK, PLL108);
 	else if (x = 1280 && y = 1024)
-    	dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CLK, PLL108);
+		dispControl = FIELD_SET(dispControl,
+					CRT_DISPLAY_CTRL, CLK, PLL108);
 	else /* default to VGA clock */
-    	dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CLK, PLL25);
+		dispControl = FIELD_SET(dispControl,
+					CRT_DISPLAY_CTRL, CLK, PLL25);
 
 	/* Set bit 25:24 of display controller */
     dispControl = FIELD_SET(dispControl, CRT_DISPLAY_CTRL, CRTSELECT, CRT);
-- 
2.1.0


^ permalink raw reply related

* Re: [PATCH] [media] ivtv: use arch_phys_wc_add() and require PAT disabled
From: Andy Walls @ 2015-04-25 11:12 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: linux-media, luto, mst, linux-kernel, linux-fbdev,
	Luis R. Rodriguez, Mauro Carvalho Chehab, Suresh Siddha,
	Ingo Molnar, Thomas Gleixner, Juergen Gross, Daniel Vetter,
	Dave Airlie, Bjorn Helgaas, Antonino Daplas,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Dave Hansen,
	Arnd Bergmann, Stefan Bader, Ville Syrjälä, Mel Gorman,
	Vlastimil Babka, Borislav Petkov, Davidlohr Bueso, konrad.wilk,
	ville.syrjala, david.vrabel, jbeulich, toshi.kani,
	Roger Pau Monné, ivtv-devel, xen-devel
In-Reply-To: <1429731182-6974-1-git-send-email-mcgrof@do-not-panic.com>

Hi Luis,

Sorry for the late reply.

Thank you for the patch! See my comments below:

On Wed, 2015-04-22 at 12:33 -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
> 
> We are burrying direct access to MTRR code support on
> x86 in order to take advantage of PAT. In the future we
> also want to make the default behaviour of ioremap_nocache()
> to use strong UC, use of mtrr_add() on those systems
> would make write-combining void.
> 
> In order to help both enable us to later make strong
> UC default and in order to phase out direct MTRR access
> code port the driver over to arch_phys_wc_add() and
> annotate that the device driver requires systems to
> boot with PAT disabled, with the nopat kernel parameter.
> 
> This is a worthy comprmise given that the hardware is
> really rare these days,

I'm OK with the compromise solution.  It makes sense.

>  and perhaps only some lost souls
> in some third world country are expected to be using this
> feature of the device driver.
> 
> Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Andy Walls <awalls@md.metrocast.net>
> Cc: Suresh Siddha <sbsiddha@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Antonino Daplas <adaplas@gmail.com>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Stefan Bader <stefan.bader@canonical.com>
> Cc: Ville Syrjälä <syrjala@sci.fi>
> Cc: Mel Gorman <mgorman@suse.de>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Davidlohr Bueso <dbueso@suse.de>
> Cc: konrad.wilk@oracle.com
> Cc: ville.syrjala@linux.intel.com
> Cc: david.vrabel@citrix.com
> Cc: jbeulich@suse.com
> Cc: toshi.kani@hp.com
> Cc: Roger Pau Monné <roger.pau@citrix.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: ivtv-devel@ivtvdriver.org
> Cc: linux-media@vger.kernel.org
> Cc: xen-devel@lists.xensource.com
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> ---
>  drivers/media/pci/ivtv/Kconfig  |  3 +++
>  drivers/media/pci/ivtv/ivtvfb.c | 59 +++++++++++++++++------------------------
>  2 files changed, 27 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
> index dd6ee57e..b2a7f88 100644
> --- a/drivers/media/pci/ivtv/Kconfig
> +++ b/drivers/media/pci/ivtv/Kconfig
> @@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
>  	  This is used in the Hauppauge PVR-350 card. There is a driver
>  	  homepage at <http://www.ivtvdriver.org>.
>  
> +	  If you have this hardware you will need to boot with PAT disabled
> +	  on your x86 systems, use the nopat kernel parameter.
> +
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ivtvfb.
> diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
> index 9ff1230..552408b 100644
> --- a/drivers/media/pci/ivtv/ivtvfb.c
> +++ b/drivers/media/pci/ivtv/ivtvfb.c
> @@ -44,8 +44,8 @@
>  #include <linux/ivtvfb.h>
>  #include <linux/slab.h>
>  
> -#ifdef CONFIG_MTRR
> -#include <asm/mtrr.h>
> +#ifdef CONFIG_X86_64
> +#include <asm/pat.h>
>  #endif
>  
>  #include "ivtv-driver.h"
> @@ -155,12 +155,11 @@ struct osd_info {
>  	/* Buffer size */
>  	u32 video_buffer_size;
>  
> -#ifdef CONFIG_MTRR
>  	/* video_base rounded down as required by hardware MTRRs */
>  	unsigned long fb_start_aligned_physaddr;
>  	/* video_base rounded up as required by hardware MTRRs */
>  	unsigned long fb_end_aligned_physaddr;
> -#endif
> +	int wc_cookie;
>  
>  	/* Store the buffer offset */
>  	int set_osd_coords_x;
> @@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
>  static int ivtvfb_init_io(struct ivtv *itv)
>  {
>  	struct osd_info *oi = itv->osd_info;
> +	/* Find the largest power of two that maps the whole buffer */
> +	int size_shift = 31;
>  
>  	mutex_lock(&itv->serialize_lock);
>  	if (ivtv_init_on_first_open(itv)) {
> @@ -1120,6 +1121,7 @@ static int ivtvfb_init_io(struct ivtv *itv)
>  	oi->video_buffer_size = 1704960;
>  
>  	oi->video_pbase = itv->base_addr + IVTV_DECODER_OFFSET + oi->video_rbase;
> +	/* XXX: split this for PAT */

Please remove this comment.  It is prescriptive of a particular
solution, and probably not the one I'm going to implement.  The ivtv
main driver alreay splits the encoder, decoder, and register regions
into 3 mappings.  The final solution will set the whole 8 MB decoder
region mapping to WC, and then fix up all calls in the ivtvfb and ivtv
drivers where writes to the decoder memory with WC enabled could be a
problem.

Also many other places in the driver need audit in a conversion to work
with PAT, so no need to call out this one location with a comment.

>  	oi->video_vbase = itv->dec_mem + oi->video_rbase;
>  
>  	if (!oi->video_vbase) {
> @@ -1132,29 +1134,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
>  			oi->video_pbase, oi->video_vbase,
>  			oi->video_buffer_size / 1024);
>  
> -#ifdef CONFIG_MTRR
> -	{
> -		/* Find the largest power of two that maps the whole buffer */
> -		int size_shift = 31;
> -
> -		while (!(oi->video_buffer_size & (1 << size_shift))) {
> -			size_shift--;
> -		}
> -		size_shift++;
> -		oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << size_shift) - 1);
> -		oi->fb_end_aligned_physaddr = oi->video_pbase + oi->video_buffer_size;
> -		oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
> -		oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
> -		if (mtrr_add(oi->fb_start_aligned_physaddr,
> -			oi->fb_end_aligned_physaddr - oi->fb_start_aligned_physaddr,
> -			     MTRR_TYPE_WRCOMB, 1) < 0) {
> -			IVTVFB_INFO("disabled mttr\n");
> -			oi->fb_start_aligned_physaddr = 0;
> -			oi->fb_end_aligned_physaddr = 0;
> -		}
> -	}
> -#endif
> -
> +	while (!(oi->video_buffer_size & (1 << size_shift)))
> +		size_shift--;
> +	size_shift++;
> +	oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << size_shift) - 1);
> +	oi->fb_end_aligned_physaddr = oi->video_pbase + oi->video_buffer_size;
> +	oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
> +	oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
> +	oi->wc_cookie = arch_phys_wc_add(oi->fb_start_aligned_physaddr,
> +					 oi->fb_end_aligned_physaddr -
> +					 oi->fb_start_aligned_physaddr);
>  	/* Blank the entire osd. */
>  	memset_io(oi->video_vbase, 0, oi->video_buffer_size);
>  
> @@ -1172,14 +1161,7 @@ static void ivtvfb_release_buffers (struct ivtv *itv)
>  
>  	/* Release pseudo palette */
>  	kfree(oi->ivtvfb_info.pseudo_palette);
> -
> -#ifdef CONFIG_MTRR
> -	if (oi->fb_end_aligned_physaddr) {
> -		mtrr_del(-1, oi->fb_start_aligned_physaddr,
> -			oi->fb_end_aligned_physaddr - oi->fb_start_aligned_physaddr);
> -	}
> -#endif
> -
> +	arch_phys_wc_del(oi->wc_cookie);
>  	kfree(oi);
>  	itv->osd_info = NULL;
>  }
> @@ -1190,6 +1172,13 @@ static int ivtvfb_init_card(struct ivtv *itv)
>  {
>  	int rc;
>  
> +#ifdef CONFIG_X86_64
> +	if (WARN(pat_enabled,

This check might be better placed in ivtvfb_init().  This check is going
to have the same result for every PVR-350 card in the system that is
found by ivtvfb.

> +		 "ivtv needs PAT disabled, boot with nopat kernel parameter\n")) {

This needs to read "ivtvfb needs [...]" to avoid user confusion with the
main ivtv driver module.

This change is the only one I really care about.  Then I can give my
Ack.

Regards,
Andy

> +		return EINVAL;
> +	}
> +#endif
> +
>  	if (itv->osd_info) {
>  		IVTVFB_ERR("Card %d already initialised\n", ivtvfb_card_id);
>  		return -EBUSY;



^ permalink raw reply

* Re: simple framebuffer slower by factor of 20, on socfpga (arm) platform
From: Pavel Machek @ 2015-04-26 19:31 UTC (permalink / raw)
  To: Geert Uytterhoeven, Russell King
  Cc: Tomi Valkeinen, Archit Taneja, Marek Vasut, kernel list,
	Dinh Nguyen, Jean-Christophe PLAGNIOL-VILLARD, Grant Likely,
	Rob Herring, Jingoo Han, Rob Clark, Linux Fbdev development list,
	devicetree@vger.kernel.org, Alexander Shiyan, Russell King,
	H Hartley Sweeten
In-Reply-To: <CAMuHMdV+-VcZD5TMswUjwW0fk=-76oYimeKJ_P9QC5Md4De-JA@mail.gmail.com>

On Fri 2015-04-24 15:46:56, Geert Uytterhoeven wrote:
> On Fri, Apr 24, 2015 at 3:40 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On 24/04/15 16:29, Pavel Machek wrote:
> >> On Fri 2015-04-10 12:35:52, Archit Taneja wrote:
> >>>> That said, if the fb is in RAM, and is only written by the CPU, I think
> >>>> a normal memcpy() for fb_memcpy_fromfb() should be fine...
> >>>
> >>> I didn't test for performance regressions when I posted this patch.
> >>>
> >>> A look at _memcpy_fromio in arch/arm/kernel/io.c shows that readb() is used
> >>> all the time, even when the source and destination addresses are aligned for
> >>> larger reads to be possible. Other archs seem to use readl() or readq() when
> >>> they can. Maybe that makes memcpy_fromio slower than the implementation of
> >>> memcpy on arm?
> >>
> >> Ok, can you prepare a patch for me to try? Or should we just revert
> >> the original commit?
> >
> > The old way worked fine, afaik, so maybe we can revert. But still, isn't
> > it more correct to use memcpy_fromio? It's (possibly) io memory we have
> > here.
> 
> Yes it is.
> 
> So please optimize ARM's _memcpy_fromio(), _memcpy_toio(), and _memset_io().
> That will benefit other drivers on ARM, too.

No, sorry. Yes, I could "optimize" memcpy_toio... just by sticking
memcpy there, as for example asm-generic/io.h suggests.

Maybe it would break something. Maybe not, but potential for that
clearly is there... since this is very seldom used function. Or do you
know drivers that would benefit from this?

void _memcpy_toio(volatile void __iomem *to, const void *from, size_t count)
{
        const unsigned char *f = from;
	while (count) {
	      count--;
	      writeb(*f, to);
	      f++;
	      to++;
 	}
}

We have a regression, we have a patch that causes the
regression. Right fix at this point is to revert a "cleanup" that
causes this, not try to "optimize" otherwise unused piece of code.

commit 981409b25e2a99409b26daa67293ca1cfd5ea0a0
Author: Archit Taneja <archit@ti.com>
Date:   Fri Nov 16 14:46:04 2012 +0530

    fbdev: arm has __raw I/O accessors, use them in fb.h

    This removes the sparse warnings on arm platforms:

    warning: cast removes address space of expression

Signed-off-by: Archit Taneja <archit@ti.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: H Hartley Sweeten <hsweeten at visionengravers.com>
Cc: Alexander Shiyan <shc_work@mail.ru>
Cc: Russell King <linux@arm.linux.org.uk>

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* [PATCH v2] ivtv: use arch_phys_wc_add() and require PAT disabled
From: Luis R. Rodriguez @ 2015-04-27 16:43 UTC (permalink / raw)
  To: andy, awalls, linux-media
  Cc: luto, mst, linux-kernel, linux-fbdev, Luis R. Rodriguez,
	Mauro Carvalho Chehab, Suresh Siddha, Ingo Molnar,
	Thomas Gleixner, Juergen Gross, Daniel Vetter, Dave Airlie,
	Bjorn Helgaas, Antonino Daplas, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen, Dave Hansen, Arnd Bergmann, Stefan Bader,
	Ville Syrjälä, Mel Gorman, Vlastimil Babka,
	Borislav Petkov, Davidlohr Bueso, konrad.wilk, ville.syrjala,
	david.vrabel, jbeulich, toshi.kani, Roger Pau Monné,
	ivtv-devel, xen-devel

From: "Luis R. Rodriguez" <mcgrof@suse.com>

We are burrying direct access to MTRR code support on
x86 in order to take advantage of PAT. In the future we
also want to make the default behaviour of ioremap_nocache()
to use strong UC, use of mtrr_add() on those systems
would make write-combining void.

In order to help both enable us to later make strong
UC default and in order to phase out direct MTRR access
code port the driver over to arch_phys_wc_add() and
annotate that the device driver requires systems to
boot with PAT disabled, with the nopat kernel parameter.

This is a worthy comprmise given that the hardware is
really rare these days, and perhaps only some lost souls
in some third world country are expected to be using this
feature of the device driver.

Cc: Andy Walls <awalls@md.metrocast.net>
Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Stefan Bader <stefan.bader@canonical.com>
Cc: Ville Syrjälä <syrjala@sci.fi>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Borislav Petkov <bp@suse.de>
Cc: Davidlohr Bueso <dbueso@suse.de>
Cc: konrad.wilk@oracle.com
Cc: ville.syrjala@linux.intel.com
Cc: david.vrabel@citrix.com
Cc: jbeulich@suse.com
Cc: toshi.kani@hp.com
Cc: Roger Pau Monné <roger.pau@citrix.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: ivtv-devel@ivtvdriver.org
Cc: linux-media@vger.kernel.org
Cc: xen-devel@lists.xensource.com
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---

This v2 moves the PAT bail out error check on to ivtvfb_init()
as per Andy's request. It also removes some comment about TODO
items for PAT.

 drivers/media/pci/ivtv/Kconfig  |  3 +++
 drivers/media/pci/ivtv/ivtvfb.c | 58 ++++++++++++++++-------------------------
 2 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
index dd6ee57e..b2a7f88 100644
--- a/drivers/media/pci/ivtv/Kconfig
+++ b/drivers/media/pci/ivtv/Kconfig
@@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
 	  This is used in the Hauppauge PVR-350 card. There is a driver
 	  homepage at <http://www.ivtvdriver.org>.
 
+	  If you have this hardware you will need to boot with PAT disabled
+	  on your x86 systems, use the nopat kernel parameter.
+
 	  To compile this driver as a module, choose M here: the
 	  module will be called ivtvfb.
diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index 9ff1230..8761e3e 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -44,8 +44,8 @@
 #include <linux/ivtvfb.h>
 #include <linux/slab.h>
 
-#ifdef CONFIG_MTRR
-#include <asm/mtrr.h>
+#ifdef CONFIG_X86_64
+#include <asm/pat.h>
 #endif
 
 #include "ivtv-driver.h"
@@ -155,12 +155,11 @@ struct osd_info {
 	/* Buffer size */
 	u32 video_buffer_size;
 
-#ifdef CONFIG_MTRR
 	/* video_base rounded down as required by hardware MTRRs */
 	unsigned long fb_start_aligned_physaddr;
 	/* video_base rounded up as required by hardware MTRRs */
 	unsigned long fb_end_aligned_physaddr;
-#endif
+	int wc_cookie;
 
 	/* Store the buffer offset */
 	int set_osd_coords_x;
@@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
 static int ivtvfb_init_io(struct ivtv *itv)
 {
 	struct osd_info *oi = itv->osd_info;
+	/* Find the largest power of two that maps the whole buffer */
+	int size_shift = 31;
 
 	mutex_lock(&itv->serialize_lock);
 	if (ivtv_init_on_first_open(itv)) {
@@ -1132,29 +1133,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
 			oi->video_pbase, oi->video_vbase,
 			oi->video_buffer_size / 1024);
 
-#ifdef CONFIG_MTRR
-	{
-		/* Find the largest power of two that maps the whole buffer */
-		int size_shift = 31;
-
-		while (!(oi->video_buffer_size & (1 << size_shift))) {
-			size_shift--;
-		}
-		size_shift++;
-		oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << size_shift) - 1);
-		oi->fb_end_aligned_physaddr = oi->video_pbase + oi->video_buffer_size;
-		oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
-		oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
-		if (mtrr_add(oi->fb_start_aligned_physaddr,
-			oi->fb_end_aligned_physaddr - oi->fb_start_aligned_physaddr,
-			     MTRR_TYPE_WRCOMB, 1) < 0) {
-			IVTVFB_INFO("disabled mttr\n");
-			oi->fb_start_aligned_physaddr = 0;
-			oi->fb_end_aligned_physaddr = 0;
-		}
-	}
-#endif
-
+	while (!(oi->video_buffer_size & (1 << size_shift)))
+		size_shift--;
+	size_shift++;
+	oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << size_shift) - 1);
+	oi->fb_end_aligned_physaddr = oi->video_pbase + oi->video_buffer_size;
+	oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
+	oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
+	oi->wc_cookie = arch_phys_wc_add(oi->fb_start_aligned_physaddr,
+					 oi->fb_end_aligned_physaddr -
+					 oi->fb_start_aligned_physaddr);
 	/* Blank the entire osd. */
 	memset_io(oi->video_vbase, 0, oi->video_buffer_size);
 
@@ -1172,14 +1160,7 @@ static void ivtvfb_release_buffers (struct ivtv *itv)
 
 	/* Release pseudo palette */
 	kfree(oi->ivtvfb_info.pseudo_palette);
-
-#ifdef CONFIG_MTRR
-	if (oi->fb_end_aligned_physaddr) {
-		mtrr_del(-1, oi->fb_start_aligned_physaddr,
-			oi->fb_end_aligned_physaddr - oi->fb_start_aligned_physaddr);
-	}
-#endif
-
+	arch_phys_wc_del(oi->wc_cookie);
 	kfree(oi);
 	itv->osd_info = NULL;
 }
@@ -1284,6 +1265,13 @@ static int __init ivtvfb_init(void)
 	int registered = 0;
 	int err;
 
+#ifdef CONFIG_X86_64
+	if (WARN(pat_enabled,
+		 "ivtvfb needs PAT disabled, boot with nopat kernel parameter\n")) {
+		return EINVAL;
+	}
+#endif
+
 	if (ivtvfb_card_id < -1 || ivtvfb_card_id >= IVTV_MAX_CARDS) {
 		printk(KERN_ERR "ivtvfb:  ivtvfb_card_id parameter is out of range (valid range: -1 - %d)\n",
 		     IVTV_MAX_CARDS - 1);
-- 
2.3.2.209.gd67f9d5.dirty


^ permalink raw reply related

* Re: [PATCH] [media] ivtv: use arch_phys_wc_add() and require PAT disabled
From: Luis R. Rodriguez @ 2015-04-27 16:43 UTC (permalink / raw)
  To: Andy Walls
  Cc: Luis R. Rodriguez, linux-media, luto, mst, linux-kernel,
	linux-fbdev, Mauro Carvalho Chehab, Suresh Siddha, Ingo Molnar,
	Thomas Gleixner, Juergen Gross, Daniel Vetter, Dave Airlie,
	Bjorn Helgaas, Antonino Daplas, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen, Dave Hansen, Arnd Bergmann, Stefan Bader,
	Ville Syrjälä, Mel Gorman, Vlastimil Babka,
	Borislav Petkov, Davidlohr Bueso, konrad.wilk, ville.syrjala,
	david.vrabel, jbeulich, toshi.kani, Roger Pau Monné,
	ivtv-devel, xen-devel
In-Reply-To: <1429960325.2109.13.camel@palomino.walls.org>

On Sat, Apr 25, 2015 at 07:12:05AM -0400, Andy Walls wrote:
> Hi Luis,
> 
> Sorry for the late reply.
> 
> Thank you for the patch! See my comments below:
> 
> On Wed, 2015-04-22 at 12:33 -0700, Luis R. Rodriguez wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> > 
> > We are burrying direct access to MTRR code support on
> > x86 in order to take advantage of PAT. In the future we
> > also want to make the default behaviour of ioremap_nocache()
> > to use strong UC, use of mtrr_add() on those systems
> > would make write-combining void.
> > 
> > In order to help both enable us to later make strong
> > UC default and in order to phase out direct MTRR access
> > code port the driver over to arch_phys_wc_add() and
> > annotate that the device driver requires systems to
> > boot with PAT disabled, with the nopat kernel parameter.
> > 
> > This is a worthy comprmise given that the hardware is
> > really rare these days,
> 
> I'm OK with the compromise solution.  It makes sense.

OK great!

> > diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
> > index 9ff1230..552408b 100644
> > --- a/drivers/media/pci/ivtv/ivtvfb.c
> > +++ b/drivers/media/pci/ivtv/ivtvfb.c
> > @@ -1120,6 +1121,7 @@ static int ivtvfb_init_io(struct ivtv *itv)
> >  	oi->video_buffer_size = 1704960;
> >  
> >  	oi->video_pbase = itv->base_addr + IVTV_DECODER_OFFSET + oi->video_rbase;
> > +	/* XXX: split this for PAT */
> 
> Please remove this comment. 

Done.

> > @@ -1190,6 +1172,13 @@ static int ivtvfb_init_card(struct ivtv *itv)
> >  {
> >  	int rc;
> >  
> > +#ifdef CONFIG_X86_64
> > +	if (WARN(pat_enabled,
> 
> This check might be better placed in ivtvfb_init().  This check is going
> to have the same result for every PVR-350 card in the system that is
> found by ivtvfb.

OK moved!

> > +		 "ivtv needs PAT disabled, boot with nopat kernel parameter\n")) {
> 
> This needs to read "ivtvfb needs [...]" to avoid user confusion with the
> main ivtv driver module.

OK!

> This change is the only one I really care about.  Then I can give my
> Ack.

OK!

  Luis

^ permalink raw reply

* Re: [PATCH] IB/ipath: use arch_phys_wc_add() and require PAT disabled
From: Luis R. Rodriguez @ 2015-04-27 16:46 UTC (permalink / raw)
  To: Doug Ledford, Mike Marciniszyn, roland, Hefty, Sean,
	Hal Rosenstock, linux-rdma
  Cc: Andy Lutomirski, Michael S. Tsirkin, linux-kernel@vger.kernel.org,
	Luis R. Rodriguez, Suresh Siddha, Rickard Strandqvist,
	Mike Marciniszyn, Roland Dreier, Ingo Molnar, Linus Torvalds,
	Thomas Gleixner, Juergen Gross, Daniel Vetter, Dave Airlie,
	Bjorn Helgaas, Antonino Daplas, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen, Ville Syrjälä, Mel Gorman,
	Vlastimil Babka
In-Reply-To: <1429730795-6721-1-git-send-email-mcgrof@do-not-panic.com>

On Wed, Apr 22, 2015 at 12:26 PM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>
> We are burrying direct access to MTRR code support on
> x86 in order to take advantage of PAT. In the future we
> also want to make the default behaviour of ioremap_nocache()
> to use strong UC, use of mtrr_add() on those systems
> would make write-combining void.
>
> In order to help both enable us to later make strong
> UC default and in order to phase out direct MTRR access
> code port the driver over to arch_phys_wc_add() and
> annotate that the device driver requires systems to
> boot with PAT disabled, with the nopat kernel parameter.
>
> This is a worthy compromise given that the ipath device
> driver powers the old HTX bus cards that only work in
> AMD systems, while the newer IB/qib device driver
> powers all PCI-e cards. The ipath device driver is
> obsolete, hardware hard to find and because of this
> this its a reasonable compromise to make to require
> users of ipath to boot with nopat.

Hey folks, I realize its being discussed whether or not to remove the
driver entirely from the kernel but in the meantime, is this a
reasonable compromise ?

 Luis

^ permalink raw reply

* Re: [PATCH v2] ivtv: use arch_phys_wc_add() and require PAT disabled
From: Andy Walls @ 2015-04-27 17:31 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: awalls, linux-media, luto, mst, linux-kernel, linux-fbdev,
	Luis R. Rodriguez, Mauro Carvalho Chehab, Suresh Siddha,
	Ingo Molnar, Thomas Gleixner, Juergen Gross, Daniel Vetter,
	Dave Airlie, Bjorn Helgaas, Antonino Daplas,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Dave Hansen,
	Arnd Bergmann, Stefan Bader, Ville Syrjälä, Mel Gorman,
	Vlastimil Babka, Borislav Petkov, Davidlohr Bueso, konrad.wilk,
	ville.syrjala, david.vrabel, jbeulich, toshi.kani,
	Roger Pau Monné, ivtv-devel, xen-devel
In-Reply-To: <1430152994-17051-1-git-send-email-mcgrof@do-not-panic.com>

On Mon, 2015-04-27 at 09:43 -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
> 
> We are burrying direct access to MTRR code support on
> x86 in order to take advantage of PAT. In the future we
> also want to make the default behaviour of ioremap_nocache()
> to use strong UC, use of mtrr_add() on those systems
> would make write-combining void.
> 
> In order to help both enable us to later make strong
> UC default and in order to phase out direct MTRR access
> code port the driver over to arch_phys_wc_add() and
> annotate that the device driver requires systems to
> boot with PAT disabled, with the nopat kernel parameter.
> 
> This is a worthy comprmise given that the hardware is
> really rare these days, and perhaps only some lost souls
> in some third world country are expected to be using this
> feature of the device driver.
> 
> Cc: Andy Walls <awalls@md.metrocast.net>

Acked-by: Andy Walls <awalls@md.metrocast.net>

Regards,
Andy

> Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Suresh Siddha <sbsiddha@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Antonino Daplas <adaplas@gmail.com>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Stefan Bader <stefan.bader@canonical.com>
> Cc: Ville Syrjälä <syrjala@sci.fi>
> Cc: Mel Gorman <mgorman@suse.de>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Davidlohr Bueso <dbueso@suse.de>
> Cc: konrad.wilk@oracle.com
> Cc: ville.syrjala@linux.intel.com
> Cc: david.vrabel@citrix.com
> Cc: jbeulich@suse.com
> Cc: toshi.kani@hp.com
> Cc: Roger Pau Monné <roger.pau@citrix.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: ivtv-devel@ivtvdriver.org
> Cc: linux-media@vger.kernel.org
> Cc: xen-devel@lists.xensource.com
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> ---
> 
> This v2 moves the PAT bail out error check on to ivtvfb_init()
> as per Andy's request. It also removes some comment about TODO
> items for PAT.
> 
>  drivers/media/pci/ivtv/Kconfig  |  3 +++
>  drivers/media/pci/ivtv/ivtvfb.c | 58 ++++++++++++++++-------------------------
>  2 files changed, 26 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
> index dd6ee57e..b2a7f88 100644
> --- a/drivers/media/pci/ivtv/Kconfig
> +++ b/drivers/media/pci/ivtv/Kconfig
> @@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
>  	  This is used in the Hauppauge PVR-350 card. There is a driver
>  	  homepage at <http://www.ivtvdriver.org>.
>  
> +	  If you have this hardware you will need to boot with PAT disabled
> +	  on your x86 systems, use the nopat kernel parameter.
> +
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ivtvfb.
> diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
> index 9ff1230..8761e3e 100644
> --- a/drivers/media/pci/ivtv/ivtvfb.c
> +++ b/drivers/media/pci/ivtv/ivtvfb.c
> @@ -44,8 +44,8 @@
>  #include <linux/ivtvfb.h>
>  #include <linux/slab.h>
>  
> -#ifdef CONFIG_MTRR
> -#include <asm/mtrr.h>
> +#ifdef CONFIG_X86_64
> +#include <asm/pat.h>
>  #endif
>  
>  #include "ivtv-driver.h"
> @@ -155,12 +155,11 @@ struct osd_info {
>  	/* Buffer size */
>  	u32 video_buffer_size;
>  
> -#ifdef CONFIG_MTRR
>  	/* video_base rounded down as required by hardware MTRRs */
>  	unsigned long fb_start_aligned_physaddr;
>  	/* video_base rounded up as required by hardware MTRRs */
>  	unsigned long fb_end_aligned_physaddr;
> -#endif
> +	int wc_cookie;
>  
>  	/* Store the buffer offset */
>  	int set_osd_coords_x;
> @@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
>  static int ivtvfb_init_io(struct ivtv *itv)
>  {
>  	struct osd_info *oi = itv->osd_info;
> +	/* Find the largest power of two that maps the whole buffer */
> +	int size_shift = 31;
>  
>  	mutex_lock(&itv->serialize_lock);
>  	if (ivtv_init_on_first_open(itv)) {
> @@ -1132,29 +1133,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
>  			oi->video_pbase, oi->video_vbase,
>  			oi->video_buffer_size / 1024);
>  
> -#ifdef CONFIG_MTRR
> -	{
> -		/* Find the largest power of two that maps the whole buffer */
> -		int size_shift = 31;
> -
> -		while (!(oi->video_buffer_size & (1 << size_shift))) {
> -			size_shift--;
> -		}
> -		size_shift++;
> -		oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << size_shift) - 1);
> -		oi->fb_end_aligned_physaddr = oi->video_pbase + oi->video_buffer_size;
> -		oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
> -		oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
> -		if (mtrr_add(oi->fb_start_aligned_physaddr,
> -			oi->fb_end_aligned_physaddr - oi->fb_start_aligned_physaddr,
> -			     MTRR_TYPE_WRCOMB, 1) < 0) {
> -			IVTVFB_INFO("disabled mttr\n");
> -			oi->fb_start_aligned_physaddr = 0;
> -			oi->fb_end_aligned_physaddr = 0;
> -		}
> -	}
> -#endif
> -
> +	while (!(oi->video_buffer_size & (1 << size_shift)))
> +		size_shift--;
> +	size_shift++;
> +	oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << size_shift) - 1);
> +	oi->fb_end_aligned_physaddr = oi->video_pbase + oi->video_buffer_size;
> +	oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
> +	oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
> +	oi->wc_cookie = arch_phys_wc_add(oi->fb_start_aligned_physaddr,
> +					 oi->fb_end_aligned_physaddr -
> +					 oi->fb_start_aligned_physaddr);
>  	/* Blank the entire osd. */
>  	memset_io(oi->video_vbase, 0, oi->video_buffer_size);
>  
> @@ -1172,14 +1160,7 @@ static void ivtvfb_release_buffers (struct ivtv *itv)
>  
>  	/* Release pseudo palette */
>  	kfree(oi->ivtvfb_info.pseudo_palette);
> -
> -#ifdef CONFIG_MTRR
> -	if (oi->fb_end_aligned_physaddr) {
> -		mtrr_del(-1, oi->fb_start_aligned_physaddr,
> -			oi->fb_end_aligned_physaddr - oi->fb_start_aligned_physaddr);
> -	}
> -#endif
> -
> +	arch_phys_wc_del(oi->wc_cookie);
>  	kfree(oi);
>  	itv->osd_info = NULL;
>  }
> @@ -1284,6 +1265,13 @@ static int __init ivtvfb_init(void)
>  	int registered = 0;
>  	int err;
>  
> +#ifdef CONFIG_X86_64
> +	if (WARN(pat_enabled,
> +		 "ivtvfb needs PAT disabled, boot with nopat kernel parameter\n")) {
> +		return EINVAL;
> +	}
> +#endif
> +
>  	if (ivtvfb_card_id < -1 || ivtvfb_card_id >= IVTV_MAX_CARDS) {
>  		printk(KERN_ERR "ivtvfb:  ivtvfb_card_id parameter is out of range (valid range: -1 - %d)\n",
>  		     IVTV_MAX_CARDS - 1);



^ permalink raw reply


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