* Re: [PATCH RFC v13 0/5] MPC512x DMA slave s/g support, OF DMA lookup
From: Vinod Koul @ 2014-05-22 5:10 UTC (permalink / raw)
To: Alexander Popov
Cc: devicetree, Lars-Peter Clausen, Arnd Bergmann, Gerhard Sittig,
Andy Shevchenko, dmaengine, Dan Williams, Anatolij Gustschin,
linuxppc-dev
In-Reply-To: <1400163335-29853-1-git-send-email-a13xp0p0v88@gmail.com>
On Thu, May 15, 2014 at 06:15:30PM +0400, Alexander Popov wrote:
> 2013/7/14 Gerhard Sittig <gsi@denx.de>:
> > this series
> > - introduces slave s/g support (that's support for DMA transfers which
> > involve peripherals in contrast to mem-to-mem transfers)
> > - adds device tree based lookup support for DMA channels
> > - combines floating patches and related feedback which already covered
> > several aspects of what the suggested LPB driver needs, to demonstrate
> > how integration might be done
>
> ...
>
> Changes in v12:
> A new patch (part 2/7) is added to this series.
> Part 6/7:
> - change the description of 'compatible' property according part 2/7;
> - improve the document according Gerhard's feedback;
>
> Parts 1/7, 2/7 and 4/7 have been applied by Vinod Koul and
> are excluded from v13.
>
> Changes in v13:
> A new patch (part 1/5) is added to this series.
> Part 2/5:
> - fix style issue;
> - improve comments;
You need to cc DT- list for 3 to 5 patches. Need ack before we can apply.
I suspect 4/5 should come first out of these 3.
Also in future pls use right subsystem name "dmaengine". Also why are these
tagged as RFC still?
--
~Vinod
^ permalink raw reply
* Re: [PATCH RFC v13 2/5] dma: mpc512x: add support for peripheral transfers
From: Vinod Koul @ 2014-05-22 5:08 UTC (permalink / raw)
To: Alexander Popov
Cc: Lars-Peter Clausen, Arnd Bergmann, Gerhard Sittig,
Andy Shevchenko, dmaengine, Dan Williams, Anatolij Gustschin,
linuxppc-dev
In-Reply-To: <1400163335-29853-3-git-send-email-a13xp0p0v88@gmail.com>
On Thu, May 15, 2014 at 06:15:32PM +0400, Alexander Popov wrote:
> Introduce support for slave s/g transfer preparation and the associated
> device control callback in the MPC512x DMA controller driver, which adds
> support for data transfers between memory and peripheral I/O to the
> previously supported mem-to-mem transfers.
Applied, after fixing sybstem name to "dmaengine"
--
~Vinod
^ permalink raw reply
* Re: [PATCH RFC v13 1/5] dmaengine: fix comment typo
From: Vinod Koul @ 2014-05-22 5:07 UTC (permalink / raw)
To: Alexander Popov
Cc: Lars-Peter Clausen, Arnd Bergmann, Gerhard Sittig,
Andy Shevchenko, dmaengine, Dan Williams, Anatolij Gustschin,
linuxppc-dev
In-Reply-To: <1400163335-29853-2-git-send-email-a13xp0p0v88@gmail.com>
On Thu, May 15, 2014 at 06:15:31PM +0400, Alexander Popov wrote:
> Fix comment typo.
Applied, thanks
--
~Vinod
^ permalink raw reply
* Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets
From: Anshuman Khandual @ 2014-05-22 5:08 UTC (permalink / raw)
To: Pedro Alves; +Cc: mikey, avagin, oleg, linux-kernel, michael, linuxppc-dev
In-Reply-To: <537B2F5E.4040102@redhat.com>
On 05/20/2014 04:03 PM, Pedro Alves wrote:
> On 05/20/2014 09:14 AM, Anshuman Khandual wrote:
>> On 05/19/2014 08:13 PM, Pedro Alves wrote:
>>> On 05/19/2014 12:46 PM, Anshuman Khandual wrote:
>>>
>>>>>> I couldn't actually find any arch that currently returns -ENODEV in
>>>>>> the "active" hook. I see that binfmt_elf.c doesn't handle
>>>>>> regset->active() returning < 0. Guess that may be why. Looks like
>>>>>> something that could be cleaned up, to me.
>>>>>>
>>>> Also it does not consider the return value of regset->active(t->task, regset)
>>>> (whose objective is to figure out whether we need to request regset->n number
>>>> of elements or less than that) in the subsequent call to regset->get function.
>>>
>>> Indeed.
>>>
>>> TBC, do you plan on fixing this? Otherwise ...
>>
>> Sure, thinking something like this as mentioned below. But still not sure how to use
>> the return type of -ENODEV from the function regset->active(). Right now if any
>> regset does have the active hook and it returns anything but positive value, it will
>> be ignored and the control moves to the next regset in view. This prevents the thread
>> core note type being written to the core dump.
>
> Looks to me that that's exactly what should happen for -ENODEV too. The regset
> should be ignored. If regset->active() returns -ENODEV, then the machine
> doesn't have the registers at all, so what makes sense to me is to not write the
> corresponding core note in the dump. IOW, on such a machine, the kernel
> generates a core exactly like if the support for these registers that don't
> make sense for this machine wasn't compiled in at all. And generates a core
> exactly like an older kernel that didn't know about that regset
> (which is fine for that same machine) yet.
>
All of this happen right now even without specifically checking for the return type
of -ENODEV and just checking for a positive value. I guess thats the reason they had
omitted -ENODEV in the first place.
>>
>> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
>> index aa3cb62..80672fb 100644
>> --- a/fs/binfmt_elf.c
>> +++ b/fs/binfmt_elf.c
>> @@ -1553,7 +1553,15 @@ static int fill_thread_core_info(struct elf_thread_core_info *t,
>> if (regset->core_note_type && regset->get &&
>> (!regset->active || regset->active(t->task, regset))) {
>> int ret;
>
> So, here, this ?
>
> (!regset->active || regset->active(t->task, regset) > 0)) {
>
>
>> - size_t size = regset->n * regset->size;
>> + size_t size;
>> +
>> + /* Request only the active elements in the regset */
>> + if (!regset->active)
>> + size = regset->n * regset->size;
>> + else
>> + size = regset->active(t->task, regset)
>> + * regset->size;
>> +
>
>
> I wonder if it wouldn't be cleaner to add a function like:
>
> int
> regset_active (tast *task, regseg *regset)
> {
> if (!regset->active)
> return regset->n * regset->size;
> else
> return regset->active(task, regset);
> }
>
> And then use it like
>
> if (regset->core_note_type && regset->get) {
> int size = regset_active (t->task, regset);
>
> if (size > 0) {
> ...
> }
>
Yeah this makes sense.
> Though at this point, we don't actually make use of
> the distinction between -ENODEV vs 0. Guess that's what
> we should be thinking about. Seems like there some details that
> need to be sorted out, and some verification that consumers aren't
> broken by outputting smaller notes -- e.g., ia64 makes me
> wonder that.
I agree.
>
> Maybe we should leave this for another day, and have tm_spr_active
> return 0 instead of -ENODEV when the machine doesn't have the hardware,
> or not install that hook at all. Seems like the effect will be the same,
> as the note isn't output if ->get fails.
Agree. Active hooks which return 0 in case of -ENODEV sounds good to me and shall
incorporate this in the next version.
>
>> void *data = kmalloc(size, GFP_KERNEL);
>> if (unlikely(!data))
>> return 0;
>>
>>>
>>>> Now coming to the installation of the .active hooks part for all the new regsets, it
>>>> should be pretty straight forward as well. Though its optional and used for elf_core_dump
>>>> purpose only, its worth adding them here. Example of an active function should be something
>>>> like this. The function is inexpensive as required.
>>>>
>>>> +static int tm_spr_active(struct task_struct *target,
>>>> + const struct user_regset *regset)
>>>> +{
>>>> + if (!cpu_has_feature(CPU_FTR_TM))
>>>> + return -ENODEV;
>>>
>>> ... unfortunately this will do the wrong thing.
>>
>> I am not sure whether I understand this correctly. Are you saying that its wrong to return
>> -ENODEV in this case as above ?
>
> No, sorry for not being clear. The (...)'s were connected:
>
> "do you plan on fixing this? Otherwise ... ... unfortunately
> this will do the wrong thing."
>
:)
^ permalink raw reply
* Re: [PATCH v5 4/4] powerpc/eeh: Avoid event on passed PE
From: Gavin Shan @ 2014-05-22 0:01 UTC (permalink / raw)
To: Alexander Graf
Cc: aik, Gavin Shan, kvm-ppc, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <537CA667.2070906@suse.de>
On Wed, May 21, 2014 at 03:13:11PM +0200, Alexander Graf wrote:
>
>On 21.05.14 07:03, Gavin Shan wrote:
>>If we detects frozen state on PE that has been passed to guest, we
>>needn't handle it. Instead, we rely on the guest to detect and recover
>>it. The patch avoid EEH event on the frozen passed PE so that the guest
>>can have chance to handle that.
>>
>>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>>---
>> arch/powerpc/kernel/eeh.c | 8 ++++++++
>> arch/powerpc/platforms/powernv/eeh-ioda.c | 3 ++-
>> 2 files changed, 10 insertions(+), 1 deletion(-)
>>
>>diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>>index 2aaf90e..25fd12d 100644
>>--- a/arch/powerpc/kernel/eeh.c
>>+++ b/arch/powerpc/kernel/eeh.c
>>@@ -400,6 +400,14 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
>> if (ret > 0)
>> return ret;
>>+ /*
>>+ * If the PE has been passed to guest, we won't check the
>>+ * state. Instead, let the guest handle it if the PE has
>
>What guest? The kernel doesn't care whether we use VFIO for a guest or not.
>
Ok. I'll not mention "guest" and "vfio" in next revision.
Thanks,
Gavin
>
>Alex
>
>>+ * been frozen.
>>+ */
>>+ if (eeh_pe_passed(pe))
>>+ return 0;
>>+
>> /* If we already have a pending isolation event for this
>> * slot, we know it's bad already, we don't need to check.
>> * Do this checking under a lock; as multiple PCI devices
>>diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>index 1b5982f..03a3ed2 100644
>>--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>>+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>@@ -890,7 +890,8 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
>> opal_pci_eeh_freeze_clear(phb->opal_id, frozen_pe_no,
>> OPAL_EEH_ACTION_CLEAR_FREEZE_ALL);
>> ret = EEH_NEXT_ERR_NONE;
>>- } else if ((*pe)->state & EEH_PE_ISOLATED) {
>>+ } else if ((*pe)->state & EEH_PE_ISOLATED ||
>>+ eeh_pe_passed(*pe)) {
>> ret = EEH_NEXT_ERR_NONE;
>> } else {
>> pr_err("EEH: Frozen PHB#%x-PE#%x (%s) detected\n",
>
^ permalink raw reply
* Re: [PATCH v5 3/4] drivers/vfio: EEH support for VFIO PCI device
From: Gavin Shan @ 2014-05-21 23:48 UTC (permalink / raw)
To: Alexander Graf
Cc: aik, Gavin Shan, kvm-ppc, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <537CA50E.9090404@suse.de>
On Wed, May 21, 2014 at 03:07:26PM +0200, Alexander Graf wrote:
>
>On 21.05.14 07:03, Gavin Shan wrote:
>>The patch adds new IOCTL command VFIO_EEH_OP to VFIO PCI device
>>to support EEH functionality for PCI devices, which have been
>>passed from host to guest via VFIO.
>>
>>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>>---
>> Documentation/vfio.txt | 6 +-
>> arch/powerpc/include/asm/eeh.h | 10 ++
>> arch/powerpc/kernel/eeh.c | 323 +++++++++++++++++++++++++++++++++++++++++
>> drivers/vfio/pci/vfio_pci.c | 99 ++++++++++++-
>> include/uapi/linux/vfio.h | 43 ++++++
>> 5 files changed, 474 insertions(+), 7 deletions(-)
>>
>>diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
>>index b9ca023..bb17ec7 100644
>>--- a/Documentation/vfio.txt
>>+++ b/Documentation/vfio.txt
>>@@ -305,7 +305,10 @@ faster, the map/unmap handling has been implemented in real mode which provides
>> an excellent performance which has limitations such as inability to do
>> locked pages accounting in real time.
>>-So 3 additional ioctls have been added:
>>+4) PPC64 guests detect PCI errors and recover from them via EEH RTAS services,
>>+which works on the basis of additional ioctl command VFIO_EEH_OP.
>>+
>>+So 4 additional ioctls have been added:
>> VFIO_IOMMU_SPAPR_TCE_GET_INFO - returns the size and the start
>> of the DMA window on the PCI bus.
>>@@ -316,6 +319,7 @@ So 3 additional ioctls have been added:
>> VFIO_IOMMU_DISABLE - disables the container.
>>+ VFIO_EEH_OP - EEH dependent operations
>
>Please document exactly what the ioctl does. In an ideal world, a
>VFIO user will just look at the documentation and be able to write a
>program against the API with it.
>
Ok. I'll amend it. Also, I'll split it to 5 ioctl commands in next revision.
>> The code flow from the example above should be slightly changed:
>>diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
>>index 34a2d83..93922ef 100644
>>--- a/arch/powerpc/include/asm/eeh.h
>>+++ b/arch/powerpc/include/asm/eeh.h
>>@@ -305,6 +305,16 @@ void eeh_add_device_late(struct pci_dev *);
>> void eeh_add_device_tree_late(struct pci_bus *);
>> void eeh_add_sysfs_files(struct pci_bus *);
>> void eeh_remove_device(struct pci_dev *);
>>+#ifdef CONFIG_VFIO_PCI_EEH
>>+int eeh_vfio_open(struct pci_dev *pdev);
>>+void eeh_vfio_release(struct pci_dev *pdev);
>>+int eeh_vfio_set_pe_option(struct pci_dev *pdev, int option, int *retval);
>>+int eeh_vfio_get_pe_addr(struct pci_dev *pdev, int option,
>>+ int *retval, int *info);
>>+int eeh_vfio_get_pe_state(struct pci_dev *pdev, int *retval, int *state);
>>+int eeh_vfio_reset_pe(struct pci_dev *pdev, int option, int *retval);
>>+int eeh_vfio_configure_pe(struct pci_dev *pdev, int *retval);
>>+#endif
>> /**
>> * EEH_POSSIBLE_ERROR() -- test for possible MMIO failure.
>>diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>>index 9c6b899..2aaf90e 100644
>>--- a/arch/powerpc/kernel/eeh.c
>>+++ b/arch/powerpc/kernel/eeh.c
>>@@ -1098,6 +1098,329 @@ void eeh_remove_device(struct pci_dev *dev)
>> edev->mode &= ~EEH_DEV_SYSFS;
>> }
>>+#ifdef CONFIG_VFIO_PCI_EEH
>>+int eeh_vfio_open(struct pci_dev *pdev)
>
>Why vfio? Also that config option will not be set if vfio is compiled
>as a module.
>
The interface is expected to be used by VFIO-PCI. I'll change the function
names to following ones in next revision. No "VFIO" will be seen :-)
eeh_dev_open();
eeh_dev_release();
static eeh_dev_check();
eeh_pe_set_option();
eeh_pe_get_addr();
eeh_pe_get_state();
eeh_pe_reset();
eeh_pe_configure();
Yeah, "#ifdef CONFIG_VFIO_PCI_EEH" can be removed safely in next revision.
>>+{
>>+ struct eeh_dev *edev;
>>+
>>+ /* No PCI device ? */
>>+ if (!pdev)
>>+ return -ENODEV;
>>+
>>+ /* No EEH device ? */
>>+ edev = pci_dev_to_eeh_dev(pdev);
>>+ if (!edev || !edev->pe)
>>+ return -ENODEV;
>>+
>>+ eeh_dev_set_passed(edev, true);
>>+ eeh_pe_set_passed(edev->pe, true);
>>+
>>+ return 0;
>>+}
>>+EXPORT_SYMBOL_GPL(eeh_vfio_open);
>>+
>>+void eeh_vfio_release(struct pci_dev *pdev)
>>+{
>>+ bool release_pe = true;
>>+ struct eeh_pe *pe = NULL;
>>+ struct eeh_dev *tmp, *edev;
>>+
>>+ /* No PCI device ? */
>>+ if (!pdev)
>>+ return;
>>+
>>+ /* No EEH device ? */
>>+ edev = pci_dev_to_eeh_dev(pdev);
>>+ if (!edev || !eeh_dev_passed(edev) ||
>>+ !edev->pe || !eeh_pe_passed(pe))
>>+ return;
>>+
>>+ /* Release device */
>>+ pe = edev->pe;
>>+ eeh_dev_set_passed(edev, false);
>>+
>>+ /* Release PE */
>>+ eeh_pe_for_each_dev(pe, edev, tmp) {
>>+ if (eeh_dev_passed(edev)) {
>>+ release_pe = false;
>>+ break;
>>+ }
>>+ }
>>+
>>+ if (release_pe)
>>+ eeh_pe_set_passed(pe, false);
>>+}
>>+EXPORT_SYMBOL(eeh_vfio_release);
>>+
>>+static int eeh_vfio_check_dev(struct pci_dev *pdev,
>>+ struct eeh_dev **pedev,
>>+ struct eeh_pe **ppe)
>>+{
>>+ struct eeh_dev *edev;
>>+
>>+ /* No device ? */
>>+ if (!pdev)
>>+ return -ENODEV;
>>+
>>+ edev = pci_dev_to_eeh_dev(pdev);
>>+ if (!edev || !eeh_dev_passed(edev) ||
>>+ !edev->pe || !eeh_pe_passed(edev->pe))
>>+ return -ENODEV;
>>+
>>+ if (pedev)
>>+ *pedev = edev;
>>+ if (ppe)
>>+ *ppe = edev->pe;
>>+
>>+ return 0;
>>+}
>>+
>>+int eeh_vfio_set_pe_option(struct pci_dev *pdev, int option, int *retval)
>>+{
>>+ struct eeh_dev *edev;
>>+ struct eeh_pe *pe;
>>+ int ret = 0;
>>+
>>+ /* Device existing ? */
>>+ ret = eeh_vfio_check_dev(pdev, &edev, &pe);
>>+ if (ret) {
>>+ pr_debug("%s: Cannot find device %s\n",
>>+ __func__, pdev ? pci_name(pdev) : "NULL");
>>+ *retval = -7;
>
>What are these? Please use proper kernel internal return values for
>errors. I don't want to see anything even remotely tied to RTAS in
>any of these patches.
>
It's the return value for RTAS call "ibm,set-eeh-option". Yeah, it
makes sense to return kernel internal values for errors and will
amend in next revision.
>>+ goto out;
>>+ }
>>+
>>+ /* Invalid option ? */
>>+ if (option < EEH_OPT_DISABLE ||
>>+ option > EEH_OPT_THAW_DMA) {
>
>This is quite confusing to read because it's not obvious what is in
>between these. Just make this a switch() statement that lists the
>allowed options. Gcc will be smart enough to optimize that into a
>bounds check.
>
Ok. Will use "switch()" in next revision.
>>+ pr_debug("%s: Option %d out of range (%d, %d)\n",
>>+ __func__, option, EEH_OPT_DISABLE, EEH_OPT_THAW_DMA);
>>+ *retval = -3;
>>+ ret = -EINVAL;
>>+ goto out;
>>+ }
>>+
>>+ if (option == EEH_OPT_DISABLE ||
>>+ option == EEH_OPT_ENABLE) {
>>+ *retval = 0;
>>+ } else {
>>+ if (!eeh_ops || !eeh_ops->set_option) {
>>+ *retval = -7;
>>+ ret = -ENOENT;
>>+ goto out;
>>+ }
>>+
>>+ ret = eeh_ops->set_option(pe, option);
>>+ if (ret) {
>>+ pr_debug("%s: Failure %d from backend\n",
>>+ __func__, ret);
>>+ *retval = -1;
>>+ goto out;
>>+ }
>>+
>>+ *retval = 0;
>>+ }
>>+out:
>>+ return ret;
>>+}
>>+EXPORT_SYMBOL_GPL(eeh_vfio_set_pe_option);
>>+
>>+int eeh_vfio_get_pe_addr(struct pci_dev *pdev, int option,
>>+ int *retval, int *info)
>>+{
>>+ struct pci_bus *bus;
>>+ struct eeh_dev *edev;
>>+ struct eeh_pe *pe;
>>+ int ret = 0;
>>+
>>+ /* Device existing ? */
>>+ ret = eeh_vfio_check_dev(pdev, &edev, &pe);
>>+ if (ret) {
>>+ *retval = -3;
>>+ goto out;
>>+ }
>>+
>>+ /* Invalid option ? */
>>+ if (option != 0 && option != 1) {
>
>0? 1? What? Don't these have names? And again, please use a switch()
>for this function.
>
Will have names in next revision and use "switch()", thanks.
>>+ pr_debug("%s: option %d out of range (0, 1)\n",
>>+ __func__, option);
>>+ *retval = -3;
>>+ ret = -EINVAL;
>>+ goto out;
>>+ }
>>+
>>+ /*
>>+ * Fill result according to option. We don't differentiate
>>+ * PCI bus and device dependent PE here. So all PEs are
>>+ * built in "shared" mode. Also, the PE address has the format
>>+ * of "00BBSS00".
>>+ */
>>+ if (option == 0) {
>>+ bus = eeh_pe_bus_get(pe);
>>+ if (!bus) {
>>+ *retval = -3;
>>+ ret = -ENODEV;
>>+ goto out;
>>+ }
>>+
>>+ *retval = 0;
>>+ *info = bus->number << 16;
>
>How about positive numbers for the number and negative ones for errors?
>
We needn't carry error numbers by "info" because "retval" or "ret" already
had that information :-)
>>+ } else {
>>+ *retval = 0;
>>+ *info = 1;
>>+ }
>>+out:
>>+ return ret;
>>+}
>>+EXPORT_SYMBOL_GPL(eeh_vfio_get_pe_addr);
>>+
>>+int eeh_vfio_get_pe_state(struct pci_dev *pdev, int *retval, int *state)
>>+{
>>+ struct eeh_dev *edev;
>>+ struct eeh_pe *pe;
>>+ int result, ret = 0;
>>+
>>+ /* Device existing ? */
>>+ ret = eeh_vfio_check_dev(pdev, &edev, &pe);
>>+ if (ret) {
>>+ *retval = -3;
>>+ goto out;
>>+ }
>>+
>>+ if (!eeh_ops || !eeh_ops->get_state) {
>>+ pr_debug("%s: Unsupported request\n",
>>+ __func__);
>>+ ret = -ENOENT;
>>+ *retval = -3;
>>+ goto out;
>>+ }
>>+
>>+ result = eeh_ops->get_state(pe, NULL);
>>+ if (!(result & EEH_STATE_RESET_ACTIVE) &&
>>+ (result & EEH_STATE_DMA_ENABLED) &&
>>+ (result & EEH_STATE_MMIO_ENABLED))
>>+ *state = 0;
>>+ else if (result & EEH_STATE_RESET_ACTIVE)
>>+ *state = 1;
>>+ else if (!(result & EEH_STATE_RESET_ACTIVE) &&
>>+ !(result & EEH_STATE_DMA_ENABLED) &&
>>+ !(result & EEH_STATE_MMIO_ENABLED))
>>+ *state = 2;
>>+ else if (!(result & EEH_STATE_RESET_ACTIVE) &&
>>+ (result & EEH_STATE_DMA_ENABLED) &&
>>+ !(result & EEH_STATE_MMIO_ENABLED))
>>+ *state = 4;
>>+ else
>>+ *state = 5;
>
>What are these numbers?
>
Will have names in next revision :)
>>+
>>+ *retval = 0;
>>+out:
>>+ return ret;
>>+}
>>+EXPORT_SYMBOL_GPL(eeh_vfio_get_pe_state);
>>+
>>+int eeh_vfio_reset_pe(struct pci_dev *pdev, int option, int *retval)
>>+{
>>+ struct eeh_dev *edev;
>>+ struct eeh_pe *pe;
>>+ int ret = 0;
>>+
>>+ /* Device existing ? */
>>+ ret = eeh_vfio_check_dev(pdev, &edev, &pe);
>>+ if (ret) {
>>+ *retval = -3;
>>+ goto out;
>>+ }
>>+
>>+ /* Invalid option ? */
>>+ if (option != EEH_RESET_DEACTIVATE &&
>>+ option != EEH_RESET_HOT &&
>>+ option != EEH_RESET_FUNDAMENTAL) {
>>+ pr_debug("%s: Unsupported option %d\n",
>>+ __func__, option);
>>+ ret = -EINVAL;
>>+ *retval = -3;
>>+ goto out;
>>+ }
>>+
>>+ if (!eeh_ops || !eeh_ops->set_option || !eeh_ops->reset) {
>>+ pr_debug("%s: Unsupported request\n",
>>+ __func__);
>>+ ret = -ENOENT;
>>+ *retval = -7;
>>+ goto out;
>>+ }
>>+
>>+ ret = eeh_ops->reset(pe, option);
>>+ if (ret) {
>>+ pr_debug("%s: Failure %d from backend\n",
>>+ __func__, ret);
>>+ *retval = -1;
>>+ goto out;
>>+ }
>>+
>>+ /*
>>+ * The PE is still in frozen state and we need clear that.
>>+ * It's good to clear frozen state after deassert to avoid
>>+ * messy IO access during reset, which might cause recrusive
>
>recursive
>
Thanks.
>>+ * frozen PE.
>>+ */
>>+ if (option == EEH_RESET_DEACTIVATE) {
>>+ ret = eeh_ops->set_option(pe, EEH_OPT_THAW_MMIO);
>>+ if (ret) {
>>+ pr_debug("%s: Cannot enable IO for PHB#%d-PE#%d (%d)\n",
>>+ __func__, pe->phb->global_number, pe->addr, ret);
>>+ *retval = -1;
>>+ goto out;
>>+ }
>>+
>>+ ret = eeh_ops->set_option(pe, EEH_OPT_THAW_DMA);
>>+ if (ret) {
>>+ pr_debug("%s: Cannot enable DMA for PHB#%d-PE#%d (%d)\n",
>>+ __func__, pe->phb->global_number, pe->addr, ret);
>>+ *retval = -1;
>>+ goto out;
>>+ }
>>+
>>+ eeh_pe_state_clear(pe, EEH_PE_ISOLATED);
>>+ }
>>+
>>+ *retval = 0;
>>+out:
>>+ return ret;
>>+}
>>+EXPORT_SYMBOL_GPL(eeh_vfio_reset_pe);
>>+
>>+int eeh_vfio_configure_pe(struct pci_dev *pdev, int *retval)
>>+{
>>+ struct eeh_dev *edev;
>>+ struct eeh_pe *pe;
>>+ int ret = 0;
>>+
>>+ /* Device existing ? */
>>+ ret = eeh_vfio_check_dev(pdev, &edev, &pe);
>>+ if (ret) {
>>+ *retval = -3;
>>+ goto out;
>>+ }
>>+
>>+ /*
>>+ * The access to PCI config space on VFIO device has some
>>+ * limitations. Part of PCI config space, including BAR
>>+ * registers are not readable and writable. So the guest
>>+ * should have stale values for those registers and we have
>>+ * to restore them in host side.
>
>I don't understand this comment. When is "configure_pe" called in the
>first place? Please provide proper function descriptions for each of
>these exported functions that tell someone who may want to use them
>what they do.
>
Yeah, I'll add the description here (also in Documentation/vfio.txt).
>Also, don't mention VFIO or guests in any function inside this file.
>
Ok. I'll avoid mentioning "VFIO" and "guest".
>>+ */
>>+ eeh_pe_restore_bars(pe);
>>+ *retval = 0;
>>+
>>+out:
>>+ return ret;
>>+}
>>+EXPORT_SYMBOL_GPL(eeh_vfio_configure_pe);
>>+
>>+#endif /* CONFIG_VFIO_PCI_EEH */
>>+
>> static int proc_eeh_show(struct seq_file *m, void *v)
>> {
>> if (!eeh_enabled()) {
>>diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
>>index 7ba0424..05c3dde 100644
>>--- a/drivers/vfio/pci/vfio_pci.c
>>+++ b/drivers/vfio/pci/vfio_pci.c
>>@@ -25,6 +25,9 @@
>> #include <linux/types.h>
>> #include <linux/uaccess.h>
>> #include <linux/vfio.h>
>>+#ifdef CONFIG_VFIO_PCI_EEH
>>+#include <asm/eeh.h>
>>+#endif
>> #include "vfio_pci_private.h"
>>@@ -152,32 +155,57 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev)
>> pci_restore_state(pdev);
>> }
>>+static void vfio_eeh_pci_release(struct pci_dev *pdev)
>>+{
>>+#ifdef CONFIG_VFIO_PCI_EEH
>>+ eeh_vfio_release(pdev);
>>+#endif
>>+}
>>+
>> static void vfio_pci_release(void *device_data)
>> {
>> struct vfio_pci_device *vdev = device_data;
>>- if (atomic_dec_and_test(&vdev->refcnt))
>>+ if (atomic_dec_and_test(&vdev->refcnt)) {
>>+ vfio_eeh_pci_release(vdev->pdev);
>> vfio_pci_disable(vdev);
>>+ }
>> module_put(THIS_MODULE);
>> }
>>+static int vfio_eeh_pci_open(struct pci_dev *pdev)
>>+{
>>+ int ret = 0;
>>+
>>+#ifdef CONFIG_VFIO_PCI_EEH
>>+ ret = eeh_vfio_open(pdev);
>>+#endif
>>+ return ret;
>>+}
>>+
>> static int vfio_pci_open(void *device_data)
>> {
>> struct vfio_pci_device *vdev = device_data;
>>+ int ret;
>> if (!try_module_get(THIS_MODULE))
>> return -ENODEV;
>> if (atomic_inc_return(&vdev->refcnt) == 1) {
>>- int ret = vfio_pci_enable(vdev);
>>- if (ret) {
>>- module_put(THIS_MODULE);
>>- return ret;
>>- }
>>+ ret = vfio_pci_enable(vdev);
>>+ if (ret)
>>+ goto error;
>>+
>>+ ret = vfio_eeh_pci_open(vdev->pdev);
>>+ if (ret)
>>+ goto error;
>> }
>> return 0;
>>+error:
>>+ module_put(THIS_MODULE);
>>+ return ret;
>> }
>> static int vfio_pci_get_irq_count(struct vfio_pci_device *vdev, int irq_type)
>>@@ -321,6 +349,51 @@ static int vfio_pci_for_each_slot_or_bus(struct pci_dev *pdev,
>> return walk.ret;
>> }
>>+static int vfio_eeh_pci_ioctl(struct pci_dev *pdev, struct vfio_eeh_op *info)
>
>I still don't like the idea of that multiplexing ioctl. I don't see
>any benefit in it whatsoever. Just create 5 individual ioctls with
>their own simple interfaces.
>
Ok. Will split in next revision. Thanks.
>Also, this interface has nothing to do with RTAS. So don't sneak in
>RTAS error numbers anywhere ;). It's QEMU's task to convert from
>kernel error codes to RTAS error codes.
>
Ok. Will do in next revision. Thanks for your comments and time, Alex :-)
Thanks,
Gavin
>>+{
>>+ int ret = 0;
>>+
>>+#ifdef CONFIG_VFIO_PCI_EEH
>>+ switch (info->op) {
>>+ case VFIO_EEH_OP_SET_OPTION:
>>+ ret = eeh_vfio_set_pe_option(pdev,
>>+ info->option.option,
>>+ &info->option.ret);
>>+ break;
>>+ case VFIO_EEH_OP_GET_ADDR:
>>+ ret = eeh_vfio_get_pe_addr(pdev,
>>+ info->addr.option,
>>+ &info->addr.ret,
>>+ &info->addr.info);
>>+ break;
>>+ case VFIO_EEH_OP_GET_STATE:
>>+ ret = eeh_vfio_get_pe_state(pdev,
>>+ &info->state.ret,
>>+ &info->state.reset_state);
>>+ info->state.cfg_cap = 1;
>>+ info->state.pe_unavail_info = 1000;
>>+ info->state.pe_recovery_info = 0;
>>+ break;
>>+ case VFIO_EEH_OP_PE_RESET:
>>+ ret = eeh_vfio_reset_pe(pdev,
>>+ info->reset.option,
>>+ &info->reset.ret);
>>+ break;
>>+ case VFIO_EEH_OP_PE_CONFIG:
>>+ ret = eeh_vfio_configure_pe(pdev,
>>+ &info->config.ret);
>>+ default:
>>+ ret = -EINVAL;
>>+ pr_debug("%s: Cannot handle op#%d\n",
>>+ __func__, info->op);
>>+ }
>>+#else
>>+ ret = -ENOENT;
>>+#endif
>>+
>>+ return ret;
>>+}
>>+
>> static long vfio_pci_ioctl(void *device_data,
>> unsigned int cmd, unsigned long arg)
>> {
>>@@ -682,6 +755,20 @@ hot_reset_release:
>> kfree(groups);
>> return ret;
>>+ } else if (cmd == VFIO_EEH_OP) {
>>+ struct vfio_eeh_op info;
>>+ int ret = 0;
>>+
>>+ minsz = sizeof(info);
>>+ if (copy_from_user(&info, (void __user *)arg, minsz))
>>+ return -EFAULT;
>>+ if (info.argsz < minsz)
>>+ return -EINVAL;
>>+
>>+ ret = vfio_eeh_pci_ioctl(vdev->pdev, &info);
>>+ if (copy_to_user((void __user *)arg, &info, minsz))
>>+ ret = -EFAULT;
>>+ return ret;
>> }
>> return -ENOTTY;
>>diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>>index cb9023d..518961d 100644
>>--- a/include/uapi/linux/vfio.h
>>+++ b/include/uapi/linux/vfio.h
>>@@ -455,6 +455,49 @@ struct vfio_iommu_spapr_tce_info {
>> #define VFIO_IOMMU_SPAPR_TCE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
>>+/*
>>+ * The VFIO operation struct provides way to support EEH functionality
>>+ * for PCI device that is passed from host to guest via VFIO.
>>+ */
>>+#define VFIO_EEH_OP_SET_OPTION 0
>>+#define VFIO_EEH_OP_GET_ADDR 1
>>+#define VFIO_EEH_OP_GET_STATE 2
>>+#define VFIO_EEH_OP_PE_RESET 3
>>+#define VFIO_EEH_OP_PE_CONFIG 4
>>+
>>+struct vfio_eeh_op {
>>+ __u32 argsz;
>>+ __u32 op;
>>+
>>+ union {
>>+ struct vfio_eeh_set_option {
>>+ __u32 option;
>>+ __s32 ret;
>>+ } option;
>>+ struct vfio_eeh_pe_addr {
>>+ __u32 option;
>>+ __s32 ret;
>>+ __u32 info;
>>+ } addr;
>>+ struct vfio_eeh_pe_state {
>>+ __s32 ret;
>>+ __u32 reset_state;
>>+ __u32 cfg_cap;
>>+ __u32 pe_unavail_info;
>>+ __u32 pe_recovery_info;
>>+ } state;
>>+ struct vfio_eeh_reset {
>>+ __u32 option;
>>+ __s32 ret;
>>+ } reset;
>>+ struct vfio_eeh_config {
>>+ __s32 ret;
>>+ } config;
>>+ };
>>+};
>>+
>>+#define VFIO_EEH_OP _IO(VFIO_TYPE, VFIO_BASE + 21)
>>+
>> /* ***************************************************************** */
>> #endif /* _UAPIVFIO_H */
>
>--
>To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH RESEND net-next 4/9] net: systemport: use the new fixed PHY helpers
From: Florian Fainelli @ 2014-05-21 22:02 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND..., Aida Mynzhasova,
netdev, Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., David Miller
In-Reply-To: <537D20EA.1000305@cogentembedded.com>
2014-05-21 14:55 GMT-07:00 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>:
> On 05/22/2014 01:38 AM, Florian Fainelli wrote:
>
>> of_phy_connect_fixed_link() is becoming obsolete, and also required
>> platform code to register the fixed PHYs at the specified addresses for
>> those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
>> plus of_phy_register_fixed_link() helpers to transition over the new
>> scheme.
>
>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> drivers/net/ethernet/broadcom/bcmsysport.c | 17 +++++++++++++++--
>> drivers/net/ethernet/broadcom/bcmsysport.h | 1 +
>> 2 files changed, 16 insertions(+), 2 deletions(-)
>
>
>> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c
>> b/drivers/net/ethernet/broadcom/bcmsysport.c
>> index d40c5b969e9e..dc708a888f80 100644
>> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
>> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
>> @@ -1327,8 +1327,8 @@ static int bcm_sysport_open(struct net_device *dev)
>> /* Read CRC forward */
>> priv->crc_fwd = !!(umac_readl(priv, UMAC_CMD) & CMD_CRC_FWD);
>>
>> - priv->phydev = of_phy_connect_fixed_link(dev,
>> bcm_sysport_adj_link,
>> -
>> priv->phy_interface);
>> + priv->phydev = of_phy_connect(dev, priv->phy_dn,
>> bcm_sysport_adj_link,
>> + 0, priv->phy_interface);
>
>
> The continuation line should start on the next character after ( on the
> above line, according to the networking coding style.
Unless I am once again not following the coding style, the patch in
patchwork has this correctly, and so does my file locally:
http://patchwork.ozlabs.org/patch/351323/
--
Florian
^ permalink raw reply
* Re: [PATCH RESEND net-next 5/9] fs_enet: use the new fixed PHY helpers
From: Sergei Shtylyov @ 2014-05-21 21:58 UTC (permalink / raw)
To: Florian Fainelli, netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND..., Aida Mynzhasova,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-6-git-send-email-f.fainelli@gmail.com>
On 05/22/2014 01:38 AM, Florian Fainelli wrote:
> of_phy_connect_fixed_link() is becoming obsolete, and also required
> platform code to register the fixed PHYs at the specified addresses for
> those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
> plus of_phy_register_fixed_link() helpers to transition over the new
> scheme.
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
> diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
> index dc80db41d6b3..d602711e00e9 100644
> --- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
> +++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
[...]
> @@ -1029,9 +1025,15 @@ static int fs_enet_probe(struct platform_device *ofdev)
> fpi->use_napi = 1;
> fpi->napi_weight = 17;
> fpi->phy_node = of_parse_phandle(ofdev->dev.of_node, "phy-handle", 0);
> - if ((!fpi->phy_node) && (!of_get_property(ofdev->dev.of_node, "fixed-link",
> - NULL)))
> - goto out_free_fpi;
> + if (!fpi->phy_node) {
> + if (of_phy_is_fixed_link(ofdev->dev.of_node)) {
These two *if* statements could be collapsed, reducing the indentation
level below.
> + err = of_phy_register_fixed_link(ofdev->dev.of_node);
> + if (err)
> + goto out_free_fpi;
> +
> + fpi->phy_node = ofdev->dev.of_node;
> + }
> + }
WBR, Sergei
^ permalink raw reply
* Re: [PATCH v5 3/4] drivers/vfio: EEH support for VFIO PCI device
From: Benjamin Herrenschmidt @ 2014-05-21 21:56 UTC (permalink / raw)
To: Alexander Graf
Cc: aik, Gavin Shan, kvm-ppc, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <537CA50E.9090404@suse.de>
On Wed, 2014-05-21 at 15:07 +0200, Alexander Graf wrote:
> > +#ifdef CONFIG_VFIO_PCI_EEH
> > +int eeh_vfio_open(struct pci_dev *pdev)
>
> Why vfio? Also that config option will not be set if vfio is compiled as
> a module.
>
> > +{
> > + struct eeh_dev *edev;
> > +
> > + /* No PCI device ? */
> > + if (!pdev)
> > + return -ENODEV;
> > +
> > + /* No EEH device ? */
> > + edev = pci_dev_to_eeh_dev(pdev);
> > + if (!edev || !edev->pe)
> > + return -ENODEV;
> > +
> > + eeh_dev_set_passed(edev, true);
> > + eeh_pe_set_passed(edev->pe, true);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(eeh_vfio_open);
Additionally, shouldn't we have some locking here ? (and in release too)
I don't like relying on the caller locking (if it does it at all).
> > + /* Device existing ? */
> > + ret = eeh_vfio_check_dev(pdev, &edev, &pe);
> > + if (ret) {
> > + pr_debug("%s: Cannot find device %s\n",
> > + __func__, pdev ? pci_name(pdev) : "NULL");
> > + *retval = -7;
>
> What are these? Please use proper kernel internal return values for
> errors. I don't want to see anything even remotely tied to RTAS in any
> of these patches.
Hint: -ENODEV
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH RESEND net-next 4/9] net: systemport: use the new fixed PHY helpers
From: Sergei Shtylyov @ 2014-05-21 21:55 UTC (permalink / raw)
To: Florian Fainelli, netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND..., Aida Mynzhasova,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-5-git-send-email-f.fainelli@gmail.com>
On 05/22/2014 01:38 AM, Florian Fainelli wrote:
> of_phy_connect_fixed_link() is becoming obsolete, and also required
> platform code to register the fixed PHYs at the specified addresses for
> those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
> plus of_phy_register_fixed_link() helpers to transition over the new
> scheme.
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/net/ethernet/broadcom/bcmsysport.c | 17 +++++++++++++++--
> drivers/net/ethernet/broadcom/bcmsysport.h | 1 +
> 2 files changed, 16 insertions(+), 2 deletions(-)
> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
> index d40c5b969e9e..dc708a888f80 100644
> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
> @@ -1327,8 +1327,8 @@ static int bcm_sysport_open(struct net_device *dev)
> /* Read CRC forward */
> priv->crc_fwd = !!(umac_readl(priv, UMAC_CMD) & CMD_CRC_FWD);
>
> - priv->phydev = of_phy_connect_fixed_link(dev, bcm_sysport_adj_link,
> - priv->phy_interface);
> + priv->phydev = of_phy_connect(dev, priv->phy_dn, bcm_sysport_adj_link,
> + 0, priv->phy_interface);
The continuation line should start on the next character after ( on the
above line, according to the networking coding style.
[...]
WBR, Sergei
^ permalink raw reply
* Re: [PATCH RESEND net-next 1/9] Documentation: devicetree: add old and deprecated 'fixed-link'
From: Sergei Shtylyov @ 2014-05-21 21:48 UTC (permalink / raw)
To: Florian Fainelli, netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND..., Aida Mynzhasova,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-2-git-send-email-f.fainelli@gmail.com>
Hello.
On 05/22/2014 01:38 AM, Florian Fainelli wrote:
> Update the fixed-link Device Tree binding documentation to contain
> information about the old and deprecated 5-digit 'fixed-link' property.
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> Documentation/devicetree/bindings/net/fixed-link.txt | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
> diff --git a/Documentation/devicetree/bindings/net/fixed-link.txt b/Documentation/devicetree/bindings/net/fixed-link.txt
> index e956de1be935..34a991eb213f 100644
> --- a/Documentation/devicetree/bindings/net/fixed-link.txt
> +++ b/Documentation/devicetree/bindings/net/fixed-link.txt
> @@ -18,6 +18,18 @@ properties:
> * 'asym-pause' (boolean, optional), to indicate that asym_pause should
> be enabled.
>
> +Old, deprecated 'fixed-link' binding:
> +
> +* A 'fixed-link' property in the EThernet MAC node, with 5 cells, of the
s/EThernet/Ethernet/
> + form <a b c d e> with the following accepted values:
> + - a: emulated phy id, choose any but but unique to the all specified
s/phy id/PHY ID/
> + fixed-links, from 0 to 31
> + - b: duplex configuration: 0 for half duplex, 1 for full duplex
> + - c: link speed in Mbits/sec, accepted values are: 10, 100 and 1000
> + - d: pause configuration: 0 for no pause, 1 for pause
> + - e: asymetric pause configuration: 0 for no asymetric pause, 1 for asymetric
Asymmetric.
WBR, Sergei
^ permalink raw reply
* [PATCH RESEND net-next 9/9] powerpc/fsl: fsl_soc: remove 'fixed-link' parsing code
From: Florian Fainelli @ 2014-05-21 21:38 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-1-git-send-email-f.fainelli@gmail.com>
Parsing and registration of fixed PHY devices was needed with the use of
of_phy_connect_fixed_link() because this function was using the
designated PHY address identifier (first cell of the property) as the
address to bind the PHY on the emulated bus.
Since commit 3be2a49e5c08d268f8af0dd4fe89a24ea8cdc339 ("of: provide a
binding for fixed link PHYs") a new pair of functions has been
introduced which allows for dynamic address allocation of these fixed
PHY devices, but also parses the old 'fixed-link' 5-digit property.
Registration of fixed PHY early in platform code was needed because we
could not issue a fixed MDIO bus re-scan within network drivers. The
fixed PHYs had to be registered before the network drivers would call
of_phy_connect_fixed_link(). All of these caveats are solved now, such
that we can safely remove of_add_fixed_phys() now.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
arch/powerpc/sysdev/fsl_soc.c | 32 --------------------------------
1 file changed, 32 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 228cf91b91c1..ffd1169ebaab 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -25,7 +25,6 @@
#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/phy.h>
-#include <linux/phy_fixed.h>
#include <linux/spi/spi.h>
#include <linux/fsl_devices.h>
#include <linux/fs_enet_pd.h>
@@ -178,37 +177,6 @@ u32 get_baudrate(void)
EXPORT_SYMBOL(get_baudrate);
#endif /* CONFIG_CPM2 */
-#ifdef CONFIG_FIXED_PHY
-static int __init of_add_fixed_phys(void)
-{
- int ret;
- struct device_node *np;
- u32 *fixed_link;
- struct fixed_phy_status status = {};
-
- for_each_node_by_name(np, "ethernet") {
- fixed_link = (u32 *)of_get_property(np, "fixed-link", NULL);
- if (!fixed_link)
- continue;
-
- status.link = 1;
- status.duplex = fixed_link[1];
- status.speed = fixed_link[2];
- status.pause = fixed_link[3];
- status.asym_pause = fixed_link[4];
-
- ret = fixed_phy_add(PHY_POLL, fixed_link[0], &status);
- if (ret) {
- of_node_put(np);
- return ret;
- }
- }
-
- return 0;
-}
-arch_initcall(of_add_fixed_phys);
-#endif /* CONFIG_FIXED_PHY */
-
#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx)
static __be32 __iomem *rstcr;
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND net-next 8/9] of: mdio: remove of_phy_connect_fixed_link
From: Florian Fainelli @ 2014-05-21 21:38 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-1-git-send-email-f.fainelli@gmail.com>
All in-tree drivers have been converted to use the new pair of
functions: of_is_fixed_phy_link() plus of_phy_register_fixed_link(), we
can now safely remove of_phy_connect_fixed_link.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/of/of_mdio.c | 38 --------------------------------------
include/linux/of_mdio.h | 10 ----------
2 files changed, 48 deletions(-)
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 1def0bb5cb37..4c1e01ed16dc 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -246,44 +246,6 @@ struct phy_device *of_phy_connect(struct net_device *dev,
EXPORT_SYMBOL(of_phy_connect);
/**
- * of_phy_connect_fixed_link - Parse fixed-link property and return a dummy phy
- * @dev: pointer to net_device claiming the phy
- * @hndlr: Link state callback for the network device
- * @iface: PHY data interface type
- *
- * This function is a temporary stop-gap and will be removed soon. It is
- * only to support the fs_enet, ucc_geth and gianfar Ethernet drivers. Do
- * not call this function from new drivers.
- */
-struct phy_device *of_phy_connect_fixed_link(struct net_device *dev,
- void (*hndlr)(struct net_device *),
- phy_interface_t iface)
-{
- struct device_node *net_np;
- char bus_id[MII_BUS_ID_SIZE + 3];
- struct phy_device *phy;
- const __be32 *phy_id;
- int sz;
-
- if (!dev->dev.parent)
- return NULL;
-
- net_np = dev->dev.parent->of_node;
- if (!net_np)
- return NULL;
-
- phy_id = of_get_property(net_np, "fixed-link", &sz);
- if (!phy_id || sz < sizeof(*phy_id))
- return NULL;
-
- sprintf(bus_id, PHY_ID_FMT, "fixed-0", be32_to_cpu(phy_id[0]));
-
- phy = phy_connect(dev, bus_id, hndlr, iface);
- return IS_ERR(phy) ? NULL : phy;
-}
-EXPORT_SYMBOL(of_phy_connect_fixed_link);
-
-/**
* of_phy_attach - Attach to a PHY without starting the state machine
* @dev: pointer to net_device claiming the phy
* @phy_np: Node pointer for the PHY
diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
index 0aa367e316cb..d449018d0726 100644
--- a/include/linux/of_mdio.h
+++ b/include/linux/of_mdio.h
@@ -22,9 +22,6 @@ extern struct phy_device *of_phy_connect(struct net_device *dev,
struct phy_device *of_phy_attach(struct net_device *dev,
struct device_node *phy_np, u32 flags,
phy_interface_t iface);
-extern struct phy_device *of_phy_connect_fixed_link(struct net_device *dev,
- void (*hndlr)(struct net_device *),
- phy_interface_t iface);
extern struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np);
@@ -59,13 +56,6 @@ static inline struct phy_device *of_phy_attach(struct net_device *dev,
return NULL;
}
-static inline struct phy_device *of_phy_connect_fixed_link(struct net_device *dev,
- void (*hndlr)(struct net_device *),
- phy_interface_t iface)
-{
- return NULL;
-}
-
static inline struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np)
{
return NULL;
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND net-next 7/9] ucc_geth: use the new fixed PHY helpers
From: Florian Fainelli @ 2014-05-21 21:38 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-1-git-send-email-f.fainelli@gmail.com>
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/freescale/ucc_geth.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index c8299c31b21f..fab39e295441 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -1728,9 +1728,6 @@ static int init_phy(struct net_device *dev)
phydev = of_phy_connect(dev, ug_info->phy_node, &adjust_link, 0,
priv->phy_interface);
- if (!phydev)
- phydev = of_phy_connect_fixed_link(dev, &adjust_link,
- priv->phy_interface);
if (!phydev) {
dev_err(&dev->dev, "Could not attach to PHY\n");
return -ENODEV;
@@ -3790,6 +3787,17 @@ static int ucc_geth_probe(struct platform_device* ofdev)
ug_info->uf_info.irq = irq_of_parse_and_map(np, 0);
ug_info->phy_node = of_parse_phandle(np, "phy-handle", 0);
+ if (!ug_info->phy_node) {
+ /* In the case of a fixed PHY, the DT node associated
+ * to the PHY is the Ethernet MAC DT node.
+ */
+ if (of_phy_is_fixed_link(np)) {
+ err = of_phy_register_fixed_link(np);
+ if (err)
+ return err;
+ }
+ ug_info->phy_node = np;
+ }
/* Find the TBI PHY node. If it's not there, we don't support SGMII */
ug_info->tbi_node = of_parse_phandle(np, "tbi-handle", 0);
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND net-next 6/9] gianfar: use the new fixed PHY helpers
From: Florian Fainelli @ 2014-05-21 21:38 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-1-git-send-email-f.fainelli@gmail.com>
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/freescale/gianfar.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index e2d42475b006..282674027c92 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -889,6 +889,17 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev)
priv->phy_node = of_parse_phandle(np, "phy-handle", 0);
+ /* In the case of a fixed PHY, the DT node associated
+ * to the PHY is the Ethernet MAC DT node.
+ */
+ if (of_phy_is_fixed_link(np)) {
+ err = of_phy_register_fixed_link(np);
+ if (err)
+ goto err_grp_init;
+
+ priv->phy_node = np;
+ }
+
/* Find the TBI PHY. If it's not there, we don't support SGMII */
priv->tbi_node = of_parse_phandle(np, "tbi-handle", 0);
@@ -1660,9 +1671,6 @@ static int init_phy(struct net_device *dev)
priv->phydev = of_phy_connect(dev, priv->phy_node, &adjust_link, 0,
interface);
- if (!priv->phydev)
- priv->phydev = of_phy_connect_fixed_link(dev, &adjust_link,
- interface);
if (!priv->phydev) {
dev_err(&dev->dev, "could not attach to PHY\n");
return -ENODEV;
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND net-next 5/9] fs_enet: use the new fixed PHY helpers
From: Florian Fainelli @ 2014-05-21 21:38 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-1-git-send-email-f.fainelli@gmail.com>
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
index dc80db41d6b3..d602711e00e9 100644
--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
@@ -792,10 +792,6 @@ static int fs_init_phy(struct net_device *dev)
phydev = of_phy_connect(dev, fep->fpi->phy_node, &fs_adjust_link, 0,
iface);
if (!phydev) {
- phydev = of_phy_connect_fixed_link(dev, &fs_adjust_link,
- iface);
- }
- if (!phydev) {
dev_err(&dev->dev, "Could not attach to PHY\n");
return -ENODEV;
}
@@ -1029,9 +1025,15 @@ static int fs_enet_probe(struct platform_device *ofdev)
fpi->use_napi = 1;
fpi->napi_weight = 17;
fpi->phy_node = of_parse_phandle(ofdev->dev.of_node, "phy-handle", 0);
- if ((!fpi->phy_node) && (!of_get_property(ofdev->dev.of_node, "fixed-link",
- NULL)))
- goto out_free_fpi;
+ if (!fpi->phy_node) {
+ if (of_phy_is_fixed_link(ofdev->dev.of_node)) {
+ err = of_phy_register_fixed_link(ofdev->dev.of_node);
+ if (err)
+ goto out_free_fpi;
+
+ fpi->phy_node = ofdev->dev.of_node;
+ }
+ }
if (of_device_is_compatible(ofdev->dev.of_node, "fsl,mpc5125-fec")) {
phy_connection_type = of_get_property(ofdev->dev.of_node,
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND net-next 4/9] net: systemport: use the new fixed PHY helpers
From: Florian Fainelli @ 2014-05-21 21:38 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-1-git-send-email-f.fainelli@gmail.com>
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 17 +++++++++++++++--
drivers/net/ethernet/broadcom/bcmsysport.h | 1 +
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index d40c5b969e9e..dc708a888f80 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -1327,8 +1327,8 @@ static int bcm_sysport_open(struct net_device *dev)
/* Read CRC forward */
priv->crc_fwd = !!(umac_readl(priv, UMAC_CMD) & CMD_CRC_FWD);
- priv->phydev = of_phy_connect_fixed_link(dev, bcm_sysport_adj_link,
- priv->phy_interface);
+ priv->phydev = of_phy_connect(dev, priv->phy_dn, bcm_sysport_adj_link,
+ 0, priv->phy_interface);
if (!priv->phydev) {
netdev_err(dev, "could not attach to PHY\n");
return -ENODEV;
@@ -1551,6 +1551,19 @@ static int bcm_sysport_probe(struct platform_device *pdev)
if (priv->phy_interface < 0)
priv->phy_interface = PHY_INTERFACE_MODE_GMII;
+ /* In the case of a fixed PHY, the DT node associated
+ * to the PHY is the Ethernet MAC DT node.
+ */
+ if (of_phy_is_fixed_link(dn)) {
+ ret = of_phy_register_fixed_link(dn);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register fixed PHY\n");
+ goto err;
+ }
+
+ priv->phy_dn = dn;
+ }
+
/* Initialize netdevice members */
macaddr = of_get_mac_address(dn);
if (!macaddr || !is_valid_ether_addr(macaddr)) {
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h
index abdeb62616df..73fd04a94797 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.h
+++ b/drivers/net/ethernet/broadcom/bcmsysport.h
@@ -656,6 +656,7 @@ struct bcm_sysport_priv {
unsigned int rx_c_index;
/* PHY device */
+ struct device_node *phy_dn;
struct phy_device *phydev;
phy_interface_t phy_interface;
int old_pause;
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND net-next 3/9] net: bcmgenet: use the new fixed PHY helpers
From: Florian Fainelli @ 2014-05-21 21:38 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-1-git-send-email-f.fainelli@gmail.com>
of_phy_connect_fixed_link() is becoming obsolete, and also required
platform code to register the fixed PHYs at the specified addresses for
those to be usable. Get rid of it and use the new of_phy_is_fixed_link()
plus of_phy_register_fixed_link() helpers to transition over the new
scheme.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/genet/bcmmii.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 4608673beaff..add8d8596084 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -298,6 +298,7 @@ int bcmgenet_mii_config(struct net_device *dev)
static int bcmgenet_mii_probe(struct net_device *dev)
{
struct bcmgenet_priv *priv = netdev_priv(dev);
+ struct device_node *dn = priv->pdev->dev.of_node;
struct phy_device *phydev;
unsigned int phy_flags;
int ret;
@@ -307,15 +308,19 @@ static int bcmgenet_mii_probe(struct net_device *dev)
return 0;
}
- if (priv->phy_dn)
- phydev = of_phy_connect(dev, priv->phy_dn,
- bcmgenet_mii_setup, 0,
- priv->phy_interface);
- else
- phydev = of_phy_connect_fixed_link(dev,
- bcmgenet_mii_setup,
- priv->phy_interface);
+ /* In the case of a fixed PHY, the DT node associated
+ * to the PHY is the Ethernet MAC DT node.
+ */
+ if (of_phy_is_fixed_link(dn)) {
+ ret = of_phy_register_fixed_link(dn);
+ if (ret)
+ return ret;
+
+ priv->phy_dn = dn;
+ }
+ phydev = of_phy_connect(dev, priv->phy_dn, bcmgenet_mii_setup, 0,
+ priv->phy_interface);
if (!phydev) {
pr_err("could not attach to PHY\n");
return -ENODEV;
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND net-next 2/9] Documentation: devicetree: net: refer to fixed-link.txt
From: Florian Fainelli @ 2014-05-21 21:38 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-1-git-send-email-f.fainelli@gmail.com>
Update the Freescale TSEC PHY, Broadcom GENET & SYSTEMPORT Device Tree
binding documentation to refer to the fixed-link Device Tree binding in
fixed-link.txt.
Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt | 2 +-
Documentation/devicetree/bindings/net/broadcom-systemport.txt | 2 +-
Documentation/devicetree/bindings/net/fsl-tsec-phy.txt | 5 +----
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt b/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
index f2febb94550e..451fef26b4df 100644
--- a/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
+++ b/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
@@ -24,7 +24,7 @@ Optional properties:
- fixed-link: When the GENET interface is connected to a MoCA hardware block or
when operating in a RGMII to RGMII type of connection, or when the MDIO bus is
voluntarily disabled, this property should be used to describe the "fixed link".
- See Documentation/devicetree/bindings/net/fsl-tsec-phy.txt for information on
+ See Documentation/devicetree/bindings/net/fixed-link.txt for information on
the property specifics
Required child nodes:
diff --git a/Documentation/devicetree/bindings/net/broadcom-systemport.txt b/Documentation/devicetree/bindings/net/broadcom-systemport.txt
index 1b7600e022dd..c183ea90d9bc 100644
--- a/Documentation/devicetree/bindings/net/broadcom-systemport.txt
+++ b/Documentation/devicetree/bindings/net/broadcom-systemport.txt
@@ -8,7 +8,7 @@ Required properties:
- local-mac-address: Ethernet MAC address (48 bits) of this adapter
- phy-mode: Should be a string describing the PHY interface to the
Ethernet switch/PHY, see Documentation/devicetree/bindings/net/ethernet.txt
-- fixed-link: see Documentation/devicetree/bindings/net/fsl-tsec-phy.txt for
+- fixed-link: see Documentation/devicetree/bindings/net/fixed-link.txt for
the property specific details
Optional properties:
diff --git a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
index 737cdef4f903..be6ea8960f20 100644
--- a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
+++ b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
@@ -42,10 +42,7 @@ Properties:
interrupt. For TSEC and eTSEC devices, the first interrupt is
transmit, the second is receive, and the third is error.
- phy-handle : See ethernet.txt file in the same directory.
- - fixed-link : <a b c d e> where a is emulated phy id - choose any,
- but unique to the all specified fixed-links, b is duplex - 0 half,
- 1 full, c is link speed - d#10/d#100/d#1000, d is pause - 0 no
- pause, 1 pause, e is asym_pause - 0 no asym_pause, 1 asym_pause.
+ - fixed-link : See fixed-link.txt in the same directory.
- phy-connection-type : See ethernet.txt file in the same directory.
This property is only really needed if the connection is of type
"rgmii-id", as all other connection types are detected by hardware.
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND net-next 1/9] Documentation: devicetree: add old and deprecated 'fixed-link'
From: Florian Fainelli @ 2014-05-21 21:38 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
In-Reply-To: <1400708331-18088-1-git-send-email-f.fainelli@gmail.com>
Update the fixed-link Device Tree binding documentation to contain
information about the old and deprecated 5-digit 'fixed-link' property.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Documentation/devicetree/bindings/net/fixed-link.txt | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/fixed-link.txt b/Documentation/devicetree/bindings/net/fixed-link.txt
index e956de1be935..34a991eb213f 100644
--- a/Documentation/devicetree/bindings/net/fixed-link.txt
+++ b/Documentation/devicetree/bindings/net/fixed-link.txt
@@ -18,6 +18,18 @@ properties:
* 'asym-pause' (boolean, optional), to indicate that asym_pause should
be enabled.
+Old, deprecated 'fixed-link' binding:
+
+* A 'fixed-link' property in the EThernet MAC node, with 5 cells, of the
+ form <a b c d e> with the following accepted values:
+ - a: emulated phy id, choose any but but unique to the all specified
+ fixed-links, from 0 to 31
+ - b: duplex configuration: 0 for half duplex, 1 for full duplex
+ - c: link speed in Mbits/sec, accepted values are: 10, 100 and 1000
+ - d: pause configuration: 0 for no pause, 1 for pause
+ - e: asymetric pause configuration: 0 for no asymetric pause, 1 for asymetric
+ pause
+
Example:
ethernet@0 {
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND 0/9 net-next] net: of_phy_connect_fixed_link removal
From: Florian Fainelli @ 2014-05-21 21:38 UTC (permalink / raw)
To: netdev
Cc: Thomas Petazzoni, open list:OPEN FIRMWARE AND...,
Florian Fainelli, Aida Mynzhasova, Sergei Shtylyov,
Richard Cochran, open list, Claudiu Manoil, Vitaly Bordug,
Grant Likely, open list:LINUX FOR POWERPC..., davem
Hi all,
This patch set removes of_phy_connect_fixed_link() from the tree now that
we have a better solution for dealing with fixed PHY (emulated PHY) devices
for drivers that require them.
First two patches update the 'fixed-link' Device Tree binding and drivers to
refere to it.
Patches 3 to 7 update the in-tree network drivers that use
of_phy_connect_fixed_link()
Patch 8 removes of_phy_connect_fixed_link
Patch 9 removes the PowerPC code that parsed the 'fixed-link' property.
Patch 9 can be merged via the net-next tree if the PowerPC folks ack it,
but it really has to be merged after the first 8 patches in order to avoid
breakage.
Florian Fainelli (9):
Documentation: devicetree: add old and deprecated 'fixed-link'
Documentation: devicetree: net: refer to fixed-link.txt
net: bcmgenet: use the new fixed PHY helpers
net: systemport: use the new fixed PHY helpers
fs_enet: use the new fixed PHY helpers
gianfar: use the new fixed PHY helpers
ucc_geth: use the new fixed PHY helpers
of: mdio: remove of_phy_connect_fixed_link
powerpc/fsl: fsl_soc: remove 'fixed-link' parsing code
.../devicetree/bindings/net/broadcom-bcmgenet.txt | 2 +-
.../bindings/net/broadcom-systemport.txt | 2 +-
.../devicetree/bindings/net/fixed-link.txt | 12 +++++++
.../devicetree/bindings/net/fsl-tsec-phy.txt | 5 +--
arch/powerpc/sysdev/fsl_soc.c | 32 ------------------
drivers/net/ethernet/broadcom/bcmsysport.c | 17 ++++++++--
drivers/net/ethernet/broadcom/bcmsysport.h | 1 +
drivers/net/ethernet/broadcom/genet/bcmmii.c | 21 +++++++-----
.../net/ethernet/freescale/fs_enet/fs_enet-main.c | 16 +++++----
drivers/net/ethernet/freescale/gianfar.c | 14 ++++++--
drivers/net/ethernet/freescale/ucc_geth.c | 14 ++++++--
drivers/of/of_mdio.c | 38 ----------------------
include/linux/of_mdio.h | 10 ------
13 files changed, 75 insertions(+), 109 deletions(-)
--
1.9.1
^ permalink raw reply
* Re: [PATCH 1/2] mtd/spi: support en25s64 device
From: Scott Wood @ 2014-05-21 20:50 UTC (permalink / raw)
To: Shengzhou Liu; +Cc: linuxppc-dev
In-Reply-To: <1400666751-11459-1-git-send-email-Shengzhou.Liu@freescale.com>
On Wed, 2014-05-21 at 18:05 +0800, Shengzhou Liu wrote:
> Add support for EON en25s64 spi device.
>
> Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com>
> ---
> drivers/mtd/devices/m25p80.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 7eda71d..6989311 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -745,6 +745,7 @@ static const struct spi_device_id m25p_ids[] = {
> { "en25q32b", INFO(0x1c3016, 0, 64 * 1024, 64, 0) },
> { "en25p64", INFO(0x1c2017, 0, 64 * 1024, 128, 0) },
> { "en25q64", INFO(0x1c3017, 0, 64 * 1024, 128, SECT_4K) },
> + { "en25s64", INFO(0x1c3817, 0, 64 * 1024, 128, 0) },
> { "en25qh256", INFO(0x1c7019, 0, 64 * 1024, 512, 0) },
>
> /* ESMT */
This needs to be sent to the mtd and/or spi maintainers, not here.
What does this have to do with patch 2/2? Don't put unrelated things in
the same patchset, especially when they're destined for different
maintainers.
-Scott
^ permalink raw reply
* Re: [PATCH V4 0/2] mm: FAULT_AROUND_ORDER patchset performance data for powerpc
From: Andrew Morton @ 2014-05-21 20:34 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: linux-arch, x86, riel, Madhavan Srinivasan, dave.hansen, peterz,
Rusty Russell, Hugh Dickins, linux-kernel, linux-mm, ak, paulus,
mgorman, linuxppc-dev, mingo
In-Reply-To: <20140521134027.263DDE009B@blue.fi.intel.com>
On Wed, 21 May 2014 16:40:27 +0300 (EEST) "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> wrote:
> > Or something. Can we please get some code commentary over
> > do_fault_around() describing this design decision and explaining the
> > reasoning behind it?
>
> I'll do this. But if do_fault_around() rework is needed, I want to do that
> first.
This sort of thing should be at least partially driven by observation
and I don't have the data for that. My seat of the pants feel is that
after the first fault, accesses at higher addresses are more
common/probable than accesses at lower addresses. In which case we
should see improvements by centering the window at some higher address
than the fault. Much instrumentation and downstream analysis is needed
and the returns will be pretty small!
But we don't need to do all that right now. Let's get the current
implementation wrapped up for 3.15: get the interface finalized (bytes,
not pages!) and get the current design decisions appropriately
documented.
^ permalink raw reply
* NUMA topology question wrt. d4edc5b6
From: Nishanth Aravamudan @ 2014-05-21 20:04 UTC (permalink / raw)
To: srivatsa.bhat; +Cc: linuxppc-dev
Hi Srivatsa,
After d4edc5b6 ("powerpc: Fix the setup of CPU-to-Node mappings during
CPU online"), cpu_to_node() looks like:
static inline int cpu_to_node(int cpu)
{
int nid;
nid = numa_cpu_lookup_table[cpu];
/*
* During early boot, the numa-cpu lookup table might not have been
* setup for all CPUs yet. In such cases, default to node 0.
*/
return (nid < 0) ? 0 : nid;
}
However, I'm curious if this is correct in all cases. I have seen
several LPARs that do not have any CPUs on node 0. In fact, because node
0 is statically set online in the initialization of the N_ONLINE
nodemask, 0 is always present to Linux, whether it is present on the
system. I'm not sure what the best thing to do here is, but I'm curious
if you have any ideas? I would like to remove the static initialization
of node 0, as it's confusing to users to see an empty node (particularly
when it's completely separate in the numbering from other nodes), but
we trip a panic (refer to:
http://www.spinics.net/lists/linux-mm/msg73321.html).
Thanks,
Nish
^ permalink raw reply
* Re: Node 0 not necessary for powerpc?
From: Nishanth Aravamudan @ 2014-05-21 19:57 UTC (permalink / raw)
To: Tejun Heo
Cc: tony.luck, linux-mm, anton, David Rientjes, Christoph Lameter,
linuxppc-dev
In-Reply-To: <20140521185812.GA5259@htj.dyndns.org>
Hi Tejun,
On 21.05.2014 [14:58:12 -0400], Tejun Heo wrote:
> Hello,
>
> On Wed, May 21, 2014 at 09:16:27AM -0500, Christoph Lameter wrote:
> > On Mon, 19 May 2014, Nishanth Aravamudan wrote:
> > > I'm seeing a panic at boot with this change on an LPAR which actually
> > > has no Node 0. Here's what I think is happening:
> > >
> > > start_kernel
> > > ...
> > > -> setup_per_cpu_areas
> > > -> pcpu_embed_first_chunk
> > > -> pcpu_fc_alloc
> > > -> ___alloc_bootmem_node(NODE_DATA(cpu_to_node(cpu), ...
> > > -> smp_prepare_boot_cpu
> > > -> set_numa_node(boot_cpuid)
> > >
> > > So we panic on the NODE_DATA call. It seems that ia64, at least, uses
> > > pcpu_alloc_first_chunk rather than embed. x86 has some code to handle
> > > early calls of cpu_to_node (early_cpu_to_node) and sets the mapping for
> > > all CPUs in setup_per_cpu_areas().
> >
> > Maybe we can switch ia64 too embed? Tejun: Why are there these
> > dependencies?
> >
> > > Thoughts? Does that mean we need something similar to x86 for powerpc?
>
> I'm missing context to properly understand what's going on but the
> specific allocator in use shouldn't matter. e.g. x86 can use both
> embed and page allocators. If the problem is that the arch is
> accessing percpu memory before percpu allocator is initialized and the
> problem was masked before somehow, the right thing to do would be
> removing those premature percpu accesses. If early percpu variables
> are really necessary, doing similar early_percpu thing as in x86 would
> be necessary.
For context: I was looking at why N_ONLINE was statically setting Node 0
to be online, whether or not the topology is that way -- I've been
getting several bugs lately where Node 0 is online, but has no CPUs and
no memory on it, on powerpc.
On powerpc, setup_per_cpu_areas calls into ___alloc_bootmem_node using
NODE_DATA(cpu_to_node(cpu)).
Currently, cpu_to_node() in arch/powerpc/include/asm/topology.h does:
/*
* During early boot, the numa-cpu lookup table might not have been
* setup for all CPUs yet. In such cases, default to node 0.
*/
return (nid < 0) ? 0 : nid;
And so early at boot, if node 0 is not present, we end up accessing an
unitialized NODE_DATA(). So this seems buggy (I'll contact the powerpc
deveopers separately on that).
I recently submitted patches to have powerpc turn on
USE_PERCPU_NUMA_NODEID and HAVE_MEMORYLESS_NODES. But then, cpu_to_node
will be accessing percpu data in setup_per_cpu_areas, which seems like a
no-no. And more specifically, since we haven't yet run
smp_prepare_boot_cpu() at this point, cpu_to_node has not yet been
initialized to provide a sane value.
Thanks,
Nish
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox