From: Corey Minyard on
Please run this through checkpatch, as it has coding style violations.

Also, this patch appears to fix bugs in addition to adding the print.
Can we have a separate patch for that?

I'm also not clear on the reason for this. I believe all this
information is already available in /proc/ipmi/<if#>/params. I don't
think there is a strong reason to print it to the log.

-corey

On 07/26/2010 08:25 PM, Yinghai Lu wrote:
> also print out the reg spacing and size for spmi and smbios.
> so bios guys could have idea to make them consistent.
>
> Signed-off-by: Yinghai Lu<yinghai(a)kernel.org>
>
> ---
> drivers/char/ipmi/ipmi_si_intf.c | 47 ++++++++++++++++++++++++++++++++-------
> 1 file changed, 39 insertions(+), 8 deletions(-)
>
> Index: linux-2.6/drivers/char/ipmi/ipmi_si_intf.c
> ===================================================================
> --- linux-2.6.orig/drivers/char/ipmi/ipmi_si_intf.c
> +++ linux-2.6/drivers/char/ipmi/ipmi_si_intf.c
> @@ -1804,9 +1804,12 @@ static int hotmod_handler(const char *va
> info->irq_setup = std_irq_setup;
> info->slave_addr = ipmb;
>
> - if (!add_smi(info))
> + if (!add_smi(info)) {
> if (try_smi_init(info))
> cleanup_one_si(info);
> + } else {
> + kfree(info);
> + }
> } else {
> /* remove */
> struct smi_info *e, *tmp_e;
> @@ -1890,9 +1893,12 @@ static __devinit void hardcode_find_bmc(
> info->irq_setup = std_irq_setup;
> info->slave_addr = slave_addrs[i];
>
> - if (!add_smi(info))
> + if (!add_smi(info)) {
> if (try_smi_init(info))
> cleanup_one_si(info);
> + } else {
> + kfree(info);
> + }
> }
> }
>
> @@ -2088,7 +2094,13 @@ static __devinit int try_init_spmi(struc
> }
> info->io.addr_data = spmi->addr.address;
>
> - add_smi(info);
> + pr_info("ipmi_si: SPMI: %s %#lx regsize %d spacing %d irq %d\n",
> + (info->io.addr_type == IPMI_IO_ADDR_SPACE) ? "io" : "mem",
> + info->io.addr_data, info->io.regsize, info->io.regspacing,
> + info->irq);
> +
> + if (add_smi(info))
> + kfree(info);
>
> return 0;
> }
> @@ -2204,7 +2216,10 @@ static int __devinit ipmi_pnp_probe(stru
> res, info->io.regsize, info->io.regspacing,
> info->irq);
>
> - return add_smi(info);
> + if (add_smi(info))
> + goto err_free;
> +
> + return 0;
>
> err_free:
> kfree(info);
> @@ -2362,7 +2377,13 @@ static __devinit void try_init_dmi(struc
> if (info->irq)
> info->irq_setup = std_irq_setup;
>
> - add_smi(info);
> + pr_info("ipmi_si: SMBIOS: %s %#lx regsize %d spacing %d irq %d\n",
> + (info->io.addr_type == IPMI_IO_ADDR_SPACE) ? "io" : "mem",
> + info->io.addr_data, info->io.regsize, info->io.regspacing,
> + info->irq);
> +
> + if (add_smi(info))
> + kfree(info);
> }
>
> static void __devinit dmi_find_bmc(void)
> @@ -2468,7 +2489,10 @@ static int __devinit ipmi_pci_probe(stru
> &pdev->resource[0], info->io.regsize, info->io.regspacing,
> info->irq);
>
> - return add_smi(info);
> + if (add_smi(info))
> + kfree(info);
> +
> + return 0;
> }
>
> static void __devexit ipmi_pci_remove(struct pci_dev *pdev)
> @@ -2581,7 +2605,12 @@ static int __devinit ipmi_of_probe(struc
>
> dev_set_drvdata(&dev->dev, info);
>
> - return add_smi(info);
> + if (add_smi(info)) {
> + kfree(info);
> + return -EBUSY;
> + }
> +
> + return 0;
> }
>
> static int __devexit ipmi_of_remove(struct of_device *dev)
> @@ -3018,6 +3047,8 @@ static __devinit void default_find_bmc(v
> info->io.addr_data);
> } else
> cleanup_one_si(info);
> + } else {
> + kfree(info);
> }
> }
> }
> @@ -3045,7 +3076,7 @@ static int add_smi(struct smi_info *new_
> si_to_str[new_smi->si_type]);
> mutex_lock(&smi_infos_lock);
> if (!is_new_interface(new_smi)) {
> - printk(KERN_CONT PFX "duplicate interface\n");
> + printk(KERN_CONT " duplicate interface\n");
> rv = -EBUSY;
> goto out_err;
> }
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Yinghai Lu on
On 07/26/2010 07:05 PM, Corey Minyard wrote:
> Please run this through checkpatch, as it has coding style violations.

yhlu(a)linux-siqj:~/xx/xx/kernel/tip/linux-2.6> ./scripts/checkpatch.pl patches/ipmi_reg_size.patch
total: 0 errors, 0 warnings, 105 lines checked

patches/ipmi_reg_size.patch has no obvious style problems and is ready for submission.
yhlu(a)linux-siqj:~/xx/xx/kernel/tip/linux-2.6>

>
> Also, this patch appears to fix bugs in addition to adding the print.
> Can we have a separate patch for that?

in the comment log, i already mentioned that.

will separate it to twol

>
> I'm also not clear on the reason for this. I believe all this
> information is already available in /proc/ipmi/<if#>/params. I don't
> think there is a strong reason to print it to the log.

then why there is printing for ACPI path and pci path?

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Corey Minyard on
On 07/26/2010 11:41 PM, Yinghai Lu wrote:
> On 07/26/2010 07:05 PM, Corey Minyard wrote:
>
>> Please run this through checkpatch, as it has coding style violations.
>>
> yhlu(a)linux-siqj:~/xx/xx/kernel/tip/linux-2.6> ./scripts/checkpatch.pl patches/ipmi_reg_size.patch
> total: 0 errors, 0 warnings, 105 lines checked
>
> patches/ipmi_reg_size.patch has no obvious style problems and is ready for submission.
> yhlu(a)linux-siqj:~/xx/xx/kernel/tip/linux-2.6>
>
Sorry, you are right.

>
>> Also, this patch appears to fix bugs in addition to adding the print.
>> Can we have a separate patch for that?
>>
> in the comment log, i already mentioned that.
>
> will separate it to twol
>
Ok, thanks

>
>> I'm also not clear on the reason for this. I believe all this
>> information is already available in /proc/ipmi/<if#>/params. I don't
>> think there is a strong reason to print it to the log.
>>
> then why there is printing for ACPI path and pci path?
>
Well, I'm not sure on this. You are right, it is printed for those
paths and not for DMI or SPMI cases. Printing too much information is
not generally a good idea, but this may be useful. I guess to make it
consistent it would be best to add this.

-corey
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/