Re: [PATCH v15 06/10] USB/ppc4xx: Add Synopsys DWC OTG HCD queue function
Hello Tirumala,
I have some coding style comments below.
On Sat, Oct 15, 2011 at 12:09 AM, tma...@apm.com wrote:
From: Tirumala Marri tma...@apm.com
Implements functions to manage Queue Heads and Queue
Transfer Descriptors of DWC USB OTG Controller.
Signed-off-by: Tirumala R Marri tma...@apm.com
Signed-off-by: Fushen Chen fc...@apm.com
Signed-off-by: Mark Miesfeld mmiesf...@apm.com
---
drivers/usb/dwc/hcd_queue.c | 696
+++
1 files changed, 696 insertions(+), 0 deletions(-)
create mode 100644 drivers/usb/dwc/hcd_queue.c
diff --git a/drivers/usb/dwc/hcd_queue.c b/drivers/usb/dwc/hcd_queue.c
new file mode 100644
index 000..67f0409
--- /dev/null
+++ b/drivers/usb/dwc/hcd_queue.c
@@ -0,0 +1,696 @@
+/*
+ * DesignWare HS OTG controller driver
+ * Copyright (C) 2006 Synopsys, Inc.
+ * Portions Copyright (C) 2010 Applied Micro Circuits Corporation.
+ *
+ * This program is free software: you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License version 2 for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see http://www.gnu.org/licenses
+ * or write to the Free Software Foundation, Inc., 51 Franklin Street,
+ * Suite 500, Boston, MA 02110-1335 USA.
+ *
+ * Based on Synopsys driver version 2.60a
+ * Modified by Mark Miesfeld mmiesf...@apm.com
+ * Modified by Stefan Roese s...@denx.de, DENX Software Engineering
+ * Modified by Chuck Meade ch...@theptrgroup.com
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS AS
IS
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING BUT NOT LIMITED TO THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL SYNOPSYS, INC. BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+/*
+ * This file contains the functions to manage Queue Heads and Queue
+ * Transfer Descriptors.
+ */
+
+#include hcd.h
+
+static inline int is_fs_ls(enum usb_device_speed speed)
+{
+ return speed == USB_SPEED_FULL || speed == USB_SPEED_LOW;
+}
+
+/* Allocates memory for a QH structure. */
+static inline struct dwc_qh *dwc_otg_hcd_qh_alloc(void)
+{
+ return kmalloc(sizeof(struct dwc_qh), GFP_ATOMIC);
+}
Why do you have this extra function? I know its inlined, but it does not
increase code readability.
+
+/**
+ * Initializes a QH structure to initialize the QH.
+ */
+#define SCHEDULE_SLOP 10
+static void dwc_otg_hcd_qh_init(struct dwc_hcd *hcd, struct dwc_qh *qh,
+ struct urb *urb)
+{
+ memset(qh, 0, sizeof(struct dwc_qh));
+
+ /* Initialize QH */
+ switch (usb_pipetype(urb-pipe)) {
+ case PIPE_CONTROL:
+ qh-ep_type = USB_ENDPOINT_XFER_CONTROL;
+ break;
+ case PIPE_BULK:
+ qh-ep_type = USB_ENDPOINT_XFER_BULK;
+ break;
+ case PIPE_ISOCHRONOUS:
+ qh-ep_type = USB_ENDPOINT_XFER_ISOC;
+ break;
+ case PIPE_INTERRUPT:
+ qh-ep_type = USB_ENDPOINT_XFER_INT;
+ break;
+ }
+
+ qh-ep_is_in = usb_pipein(urb-pipe) ? 1 : 0;
+ qh-data_toggle = DWC_OTG_HC_PID_DATA0;
+ qh-maxp = usb_maxpacket(urb-dev, urb-pipe,
!(usb_pipein(urb-pipe)));
+
+ INIT_LIST_HEAD(qh-qtd_list);
+ INIT_LIST_HEAD(qh-qh_list_entry);
+
+ qh-channel = NULL;
+ qh-speed = urb-dev-speed;
+
+ /*
+* FS/LS Enpoint on HS Hub NOT virtual root hub
+*/
+ qh-do_split = 0;
+ if (is_fs_ls(urb-dev-speed) urb-dev-tt urb-dev-tt-hub
+ urb-dev-tt-hub-devnum != 1)
+ qh-do_split = 1;
+
+ if (qh-ep_type == USB_ENDPOINT_XFER_INT ||
+ qh-ep_type == USB_ENDPOINT_XFER_ISOC) {
+ /* Compute scheduling parameters once and save them. */
+ u32 hprt;
+ int bytecount = dwc_hb_mult(qh-maxp) *
+ dwc_max_packet(qh-maxp);
+
+ qh-usecs = NS_TO_US(usb_calc_bus_time(urb-dev-speed,
+
Re: Loading Linux from already running code..
On Mon, Sep 26, 2011 at 7:26 PM, Carlos Munoz cmu...@sablenetworks.com wrote: You could build a tarball containing the root files system, dtb, Linux, plus a header indicating where the different pieces need to be loaded. Then your initial code loads the different parts at the right memory locations, sets up the Linux arguments, and jumps to Linux. That is exactly what the firmware-independent simpleImage.initrd.* targets already do. The resulting elf file include the dtb and initrd as well as a prompt-like boot arg cli. I think it will be possible to just place the simpleImage.initrd.* into you memory and let the existing code jump to the start adress of the elf image. I managed to start a ppc core using JTAG and the simpleImage.initrd elf files and I think the procedure is very much the same but instead of the JTAG debugger setting the PC to the start address, your startup code has to do it. Philipp ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Changes to of_device ?
On 06/17/2011 11:19 AM, Guillaume Dargaud wrote: [I'm on the latest Xilinx tree, FWIW, so that's 2.6.25 still ?] Not answering your primary question here, but in order to print the most recent tag (along with a uniqe suffix, see man page) that is reachable from your current branch, you can use git describe. It will print out the kernel version on your latest Xilinx tree. Greetings, Philipp ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Changes to of_device ?
On 06/17/2011 01:22 PM, Guillaume Dargaud wrote: Not answering your primary question here, but in order to print the most recent tag (along with a uniqe suffix, see man page) that is reachable from your current branch, you can use git describe. It will print out the kernel version on your latest Xilinx tree. $ git describe v2.6.37-719-gecf08a4 $ cat ./include/linux/version.h #define LINUX_VERSION_CODE 132645 #define KERNEL_VERSION(a,b,c) (((a) 16) + ((b) 8) + (c)) $ printf %x\n 132645 20625 Something doesn't quite match here... It is hexadecimal. 0x25 = 37. But my original question about how do you declare a probe function in the latest kernels still stand... The struct device_node was moved to struct device in order to make the CONFIG_OF more generic. See commit d706c1b050274b3bf97d7cb0542c0d070c9ccb8b Author: Grant Likely grant.lik...@secretlab.ca Date: Tue Apr 13 16:12:28 2010 -0700 driver-core: Add device node pointer to struct device Hope this helps, Philipp ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Changes to of_device ?
On 06/17/2011 02:57 PM, Philipp Ittershagen wrote:
The struct device_node was moved to struct device in order to make the
CONFIG_OF more generic. See
commit d706c1b050274b3bf97d7cb0542c0d070c9ccb8b
Author: Grant Likely grant.lik...@secretlab.ca
Date: Tue Apr 13 16:12:28 2010 -0700
driver-core: Add device node pointer to struct device
I'm sorry, I should have mentioned that struct of_device was replaced by
struct platform_device. So in order to get your code compiled, you have
to change the probe function to
static int xad_driver_probe(struct platform_device* dev, const struct
of_device_id *match) {
The change was introduced by
commit 2dc11581376829303b98eadb2de253bee065a56a
Author: Grant Likely grant.lik...@secretlab.ca
Date: Fri Aug 6 09:25:50 2010 -0600
of/device: Replace struct of_device with struct platform_device
Greetings,
Philipp
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Early kernel debugging
On 03/25/2011 09:50 AM, Guillaume Dargaud wrote: Hello all, what can you do when the kernel you try to run stops before printing anything on the console ? http://elinux.org/Kernel_Debugging_Tips#Debugging_early_boot_problems Basically it means connecting a debugger to the running kernel (using XMD, for example) and then reading the printk log buffer, which contains the last lines printed. Hope this helps! Philipp ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3] PPC4xx: Adding PCI(E) MSI support
Hi,
a few nitpicks here. I don't have any clue about MSI, but I have seen
some code-style related issues.
On 12/04/2010 02:33 AM, tma...@apm.com wrote:
+static int ppc4xx_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
+{
+ int err = 0;
+ int int_no = -ENOMEM;
+ unsigned int virq;
+ struct msi_msg msg;
+ struct msi_desc *entry;
+ struct ppc4xx_msi *msi_data = ppc4xx_msi;
+
+ list_for_each_entry(entry, dev-msi_list, list) {
+ int_no = msi_bitmap_alloc_hwirqs(msi_data-bitmap, 1);
+ if(int_no = 0)
+ break;
+ if(int_no 0) {
+
Empty line here not needed.
+ err = int_no;
+ pr_debug(%s: fail allocating msi interrupt\n,
+ __func__);
+ }
+ virq = irq_of_parse_and_map(msi_data-msi_dev, int_no);
+ if (virq == NO_IRQ) {
+ dev_err(dev-dev, %s: fail mapping irq\n, __func__);
+ msi_bitmap_free_hwirqs(msi_data-bitmap, int_no, 1);
+ err = -ENOSPC;
+ goto out_free;
+ }
+ msi_data-msi_virqs[int_no] = virq;
+ set_irq_data(virq, (void *)int_no);
+ dev_dbg(dev-dev, %s: virq = %d \n, __func__, virq);
+
+ /* Setup msi address space */
+ msg.address_hi = msi_data-msi_addr_hi;
+ msg.address_lo = msi_data-msi_addr_lo;
+
+ set_irq_msi(virq, entry);
+ msg.data = int_no;
+ write_msi_msg(virq, msg);
+ }
+
+out_free:
+ return err;
+}
You don't really need the goto-style here, because you have nothing to
clean up when returning. You could instead return directly and save some
lines, because you don't have to assign err and then use goto all the time.
+
+void ppc4xx_teardown_msi_irqs(struct pci_dev *dev)
+{
+ struct msi_desc *entry;
+ struct ppc4xx_msi *msi_data = ppc4xx_msi;
+
+ dev_dbg(dev-dev, PCIE-MSI: tearing down msi irqs\n);
+
+ list_for_each_entry(entry, dev-msi_list, list) {
+ if (entry-irq == NO_IRQ)
+ continue;
+ set_irq_msi(entry-irq, NULL);
+ msi_bitmap_free_hwirqs(msi_data-bitmap,
+ virq_to_hw(entry-irq), 1);
+ irq_dispose_mapping(entry-irq);
+ }
+
+ return;
This return is not needed.
+static int ppc4xx_setup_pcieh_hw(struct platform_device *dev,
+ struct resource res, struct ppc4xx_msi *msi)
+{
+ const u32 *msi_data;
+ const u32 *msi_mask;
+ const u32 *sdr_addr;
+ int err = 0;
+ dma_addr_t msi_phys;
+ void *msi_virt;
+
+ sdr_addr = of_get_property(dev-dev.of_node, sdr-base, NULL);
+ if (!sdr_addr)
+ return -1;
+
+ SDR0_WRITE(sdr_addr, (u64)res.start 32); /*HIGH addr */
+ SDR0_WRITE(sdr_addr + 1, res.start 0x); /* Low addr */
+
+
+ msi-msi_dev = of_find_node_by_name(NULL, ppc4xx-msi);
+ if (msi-msi_dev) {
+ err = -ENODEV;
+ goto error_out;
+ }
+ msi-msi_regs = of_iomap(msi-msi_dev, 0);
+ if (!msi-msi_regs) {
+ dev_err(dev-dev, of_iomap problem failed\n);
+ return -ENOMEM;
+ }
You directly return here and in all other bad cases this function
covers, you use the goto-style (which you don't really need here,
because there is no cleanup to do in case of a failure). Better use a
consistent way for returning.
+ msi_data = of_get_property(dev-dev.of_node, msi-data, NULL);
+ if (!msi_data) {
+ err = -1;
+ goto error_out;
+ }
+
+ msi_mask = of_get_property(dev-dev.of_node, msi-mask, NULL);
+ if (!msi_mask) {
+ err = -1;
+ goto error_out;
+ }
+
+ /* Program MSI Expected data and Mask bits */
+ out_be32(msi-msi_regs + PEIH_MSIED, *msi_data);
+ out_be32(msi-msi_regs + PEIH_MSIMK, *msi_mask);
+
+ return err;
+error_out:
+ return err;
+}
This was already mentioned by Josh Boyer.
+
+static int ppc4xx_of_msi_remove(struct platform_device *dev)
+{
+ struct ppc4xx_msi *msi = dev-dev.platform_data;
+ int i;
+ int virq;
+
+ for(i = 0; i NR_MSI_IRQS; i++) {
+ virq = msi-msi_virqs[i];
+ if (virq != NO_IRQ)
+ irq_dispose_mapping(virq);
+ }
+
+ if (msi-bitmap.bitmap)
+ msi_bitmap_free(msi-bitmap);
+ iounmap(msi-msi_regs);
+ of_node_put(msi-msi_dev);
+ kfree(msi);
+
+ return 0;
+}
+
+static int __devinit ppc4xx_msi_probe(struct platform_device *dev,
+ const struct of_device_id *match)
+{
+ struct ppc4xx_msi *msi;
+ struct resource res;
+ int err = 0;
+
+ msi = ppc4xx_msi;/*keep
Re: Getting the IRQ number (Was: Basic driver devel questions ?)
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 12/01/2010 05:35 PM, Guillaume Dargaud wrote: Now how do I connect the dots between the hardware definitions from the dts and my driver ? You can get the interrupt number from the dt by calling irq_of_parse_and_map(). Be sure to pass the node of your device to this function. Then you have to request the interrupt by calling request_irq. This is where you specify the interrupt handler. But first I'm not sure where to find the IRQ in there, and also I'm not sure if reading the filesystem from a module is allowed. Why do you want to read the file system? -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkz2lCEACgkQCG4q0RxCsY4GpgCgiQFRhiF7jjhUdZcUBc4Y5ScJ E0AAn0VxcCaVexepjrah64ZSS+Xhbed8 =h97e -END PGP SIGNATURE- ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH][v1] powerpc/fsl: 85xx: add cache-sram support
Hi Harninder,
On Wed, 2010-10-13 at 14:47 +0530, harninder@freescale.com wrote:
+int __init instantiate_cache_sram(struct platform_device *dev,
+ struct sram_parameters sram_params)
+{
+ if (cache_sram) {
+ dev_err(dev-dev, Already initialized cache-sram\n);
+ return -EBUSY;
+ }
+
+ cache_sram = kzalloc(sizeof(struct mpc85xx_cache_sram), GFP_KERNEL);
+ if (!cache_sram) {
+ dev_err(dev-dev, Out of memory for cache_sram structure\n);
+ return -ENOMEM;
+ }
+
+ cache_sram-base_phys = sram_params.sram_offset;
+ cache_sram-size = sram_params.sram_size;
+
+ if (!request_mem_region(cache_sram-base_phys, cache_sram-size,
+ fsl_85xx_cache_sram)) {
+ dev_err(dev-dev, %s: request memory failed\n,
+ dev-dev.of_node-full_name);
+ kfree(cache_sram);
+ return -ENXIO;
+ }
+
+ cache_sram-base_virt = ioremap_flags(cache_sram-base_phys,
+ cache_sram-size, _PAGE_COHERENT | PAGE_KERNEL);
+ if (!cache_sram-base_virt) {
+ dev_err(dev-dev, %s: ioremap_flags failed\n,
+ dev-dev.of_node-full_name);
+ release_mem_region(cache_sram-base_phys, cache_sram-size);
+ kfree(cache_sram);
+ return -ENOMEM;
+ }
+
+ cache_sram-rh = rh_create(sizeof(unsigned int));
+ if (IS_ERR(cache_sram-rh)) {
+ dev_err(dev-dev, %s: Unable to create remote heap\n,
+ dev-dev.of_node-full_name);
+ iounmap(cache_sram-base_virt);
+ release_mem_region(cache_sram-base_phys, cache_sram-size);
+ kfree(cache_sram);
+ return PTR_ERR(cache_sram-rh);
+ }
+
+ rh_attach_region(cache_sram-rh, 0, cache_sram-size);
+ spin_lock_init(cache_sram-lock);
+
+ dev_info(dev-dev, [base:0x%llx, size:0x%x] configured and loaded\n,
+ (unsigned long long)cache_sram-base_phys, cache_sram-size);
+ return 0;
+}
You could save some redundant code by using goto statements for error
handling.
For example:
...
int ret;
if (!request_mem_region(...)) {
ret = -ENXIO;
goto out_free;
}
if (!ioremap(...)) {
ret = -ENOMEM;
goto out_release;
}
if (!IS_ERR(cache_sram-rh)) {
ret = PTR_ERR(...);
goto out_unmap;
}
...
return 0;
out_unmap:
iounmap(...);
out_release:
release_mem_region(...);
out_free:
kfree(...);
return ret;
}
-- Philipp
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
