* Re: [PATCH] via-agp.c fixed compilation error and warnings for 2.6.26 [not found] ` <663ba5090810202049i16285109k34fde3ec1c9df319@mail.gmail.com> @ 2008-10-21 3:57 ` Dave Airlie 2008-10-21 4:23 ` Matthew Wilcox 0 siblings, 1 reply; 3+ messages in thread From: Dave Airlie @ 2008-10-21 3:57 UTC (permalink / raw) To: nagaraj s k Cc: trivial, greg, linux-kernel, kernel-janitors, nagaraj.krishnappa On Tue, 21 Oct 2008, nagaraj s k wrote: > Hi All, > > can you please have a look at the patch in the below mail thread, it seems > like my mails get lost often here. is there any way i can reach the > moderator or any one who can comment on my work( i know there are thousands > of patches sent across, i am not the only one). > I saw this patch and couldn't see the problem is was solving. If its just a checkpatch.pl run across the file, in parts it makes the file less readable, anything splitting stings to avoid the checkpatch.pl stupid 80 chars warnings isn't anything I'm going to worryabout. The mail says fixing compilation errors, there are no compilation errors, if it fixes checkpatch.pl issues then please say that. > > } > > } > > - printk(KERN_ERR PFX "Unknown aperture size from AGP bridge (0x%x)\n", > > temp); > > + printk(KERN_ERR PFX "Unknown aperture size from AGP > > + bridge (0x%x)\n", temp); Uglier code. > > return 0; > > } > > > > @@ -116,7 +117,8 @@ static int via_fetch_size_agp3(void) > > for (i = 0; i < agp_bridge->driver->num_aperture_sizes; i++) { > > if (temp == values[i].size_value) { > > agp_bridge->previous_size = > > - agp_bridge->current_size = (void *) (values + i); > > + agp_bridge->current_size = > > + (void *) (values + i); arguably uglier code. > > agp_bridge->aperture_size_idx = i; > > return values[i].size; > > } > > @@ -142,11 +144,12 @@ static int via_configure_agp3(void) > > > > /* 1. Enable GTLB in RX90<7>, all AGP aperture access needs to fetch > > * translation table first. > > - * 2. Enable AGP aperture in RX91<0>. This bit controls the enabling > > of the > > - * graphics AGP aperture for the AGP3.0 port. > > + * 2. Enable AGP aperture in RX91<0>. This bit controls the > > + * enabling of the graphics AGP aperture for the AGP3.0 port. this is okay. > > */ > > pci_read_config_dword(agp_bridge->dev, VIA_AGP3_GARTCTRL, &temp); > > - pci_write_config_dword(agp_bridge->dev, VIA_AGP3_GARTCTRL, temp | > > (3<<7)); > > + pci_write_config_dword(agp_bridge->dev, VIA_AGP3_GARTCTRL, > > + temp | (3<<7)); this one is probably okay. > > return 0; > > } > > > > @@ -156,7 +159,8 @@ static void via_cleanup_agp3(void) > > struct aper_size_info_16 *previous_size; > > > > previous_size = A_SIZE_16(agp_bridge->previous_size); > > - pci_write_config_byte(agp_bridge->dev, VIA_APSIZE, > > previous_size->size_value); > > + pci_write_config_byte(agp_bridge->dev, VIA_APSIZE, > > + previous_size->size_value); > > } > > > > > > @@ -165,7 +169,8 @@ static void via_tlbflush_agp3(struct agp > > u32 temp; > > > > pci_read_config_dword(agp_bridge->dev, VIA_AGP3_GARTCTRL, &temp); > > - pci_write_config_dword(agp_bridge->dev, VIA_AGP3_GARTCTRL, temp & > > ~(1<<7)); > > + pci_write_config_dword(agp_bridge->dev, VIA_AGP3_GARTCTRL, > > + temp & ~(1<<7)); > > pci_write_config_dword(agp_bridge->dev, VIA_AGP3_GARTCTRL, temp); > > } > > > > @@ -421,13 +426,13 @@ static struct agp_device_ids via_agp_dev > > * VIA's AGP3 chipsets do magick to put the AGP bridge compliant > > * with the same standards version as the graphics card. > > */ > > -static void check_via_agp3 (struct agp_bridge_data *bridge) > > +static void check_via_agp3(struct agp_bridge_data *bridge) > > { > > u8 reg; > > > > pci_read_config_byte(bridge->dev, VIA_AGPSEL, ®); > > /* Check AGP 2.0 compatibility mode. */ > > - if ((reg & (1<<1))==0) > > + if ((reg & (1<<1)) == 0) > > bridge->driver = &via_agp3_driver; > > } > > > > @@ -445,7 +450,8 @@ static int __devinit agp_via_probe(struc > > return -ENODEV; > > > > j = ent - agp_via_pci_table; > > - printk (KERN_INFO PFX "Detected VIA %s chipset\n", > > devs[j].chipset_name); > > + printk(KERN_INFO PFX "Detected VIA %s chipset\n", > > + devs[j].chipset_name); > > > > bridge = agp_alloc_bridge(); > > if (!bridge) > > @@ -461,7 +467,8 @@ static int __devinit agp_via_probe(struc > > if (pdev->device == PCI_DEVICE_ID_VIA_8367_0) { > > /* Is there a KT400 subsystem ? */ > > if (pdev->subsystem_device == PCI_DEVICE_ID_VIA_8377_0) { > > - printk(KERN_INFO PFX "Found KT400 in disguise as a KT266.\n"); > > + printk(KERN_INFO PFX "Found KT400 in disguise > > + as a KT266.\n"); uglier. > > check_via_agp3(bridge); > > } > > } > > @@ -491,8 +498,8 @@ static void __devexit agp_via_remove(str > > > > static int agp_via_suspend(struct pci_dev *pdev, pm_message_t state) > > { > > - pci_save_state (pdev); > > - pci_set_power_state (pdev, PCI_D3hot); > > + pci_save_state(pdev); > > + pci_set_power_state(pdev, PCI_D3hot); > > > > return 0; > > } > > @@ -501,7 +508,7 @@ static int agp_via_resume(struct pci_dev > > { > > struct agp_bridge_data *bridge = pci_get_drvdata(pdev); > > > > - pci_set_power_state (pdev, PCI_D0); > > + pci_set_power_state(pdev, PCI_D0); > > pci_restore_state(pdev); > > > > if (bridge->driver == &via_agp3_driver) > > > > > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] via-agp.c fixed compilation error and warnings for 2.6.26 2008-10-21 3:57 ` [PATCH] via-agp.c fixed compilation error and warnings for 2.6.26 Dave Airlie @ 2008-10-21 4:23 ` Matthew Wilcox [not found] ` <663ba5090810202137w750a1cc9m815489e92a2c6d94@mail.gmail.com> 0 siblings, 1 reply; 3+ messages in thread From: Matthew Wilcox @ 2008-10-21 4:23 UTC (permalink / raw) To: Dave Airlie Cc: nagaraj s k, trivial, greg, linux-kernel, kernel-janitors, nagaraj.krishnappa On Tue, Oct 21, 2008 at 04:57:13AM +0100, Dave Airlie wrote: > > > - printk(KERN_ERR PFX "Unknown aperture size from AGP bridge (0x%x)\n", > > > temp); > > > + printk(KERN_ERR PFX "Unknown aperture size from AGP > > > + bridge (0x%x)\n", temp); > > Uglier code. Not just that, but it won;t even compile with a recent compiler. You'd need to do it as: printk(KERN_ERR PFX "Unknown aperture size from AGP " "bridge (0x%x)\n", temp); and that would seem like not much of a win. What might be a win would be: dev_err(&agp_bridge->dev->dev, "Unknown AGP aperture size 0x%x\n", temp); > > > if (temp == values[i].size_value) { > > > agp_bridge->previous_size = > > > - agp_bridge->current_size = (void *) (values + i); > > > + agp_bridge->current_size = > > > + (void *) (values + i); > > arguably uglier code. No argument about it ... it's uglier. The (void *) cast is unnecessary. What I'd do to this function is: for (i = 0; i < agp_bridge->driver->num_aperture_sizes; i++) { if (temp != values[i].size_value) continue; agp_bridge->previous_size = agp_bridge->current_size = values + i; agp_bridge->aperture_size_idx = i; return values[i].size; } dev_err(&agp_bridge->dev->dev, "Unknown AGP aperture size 0x%x\n", temp); return 0; } > > > @@ -142,11 +144,12 @@ static int via_configure_agp3(void) > > > > > > /* 1. Enable GTLB in RX90<7>, all AGP aperture access needs to fetch > > > * translation table first. > > > - * 2. Enable AGP aperture in RX91<0>. This bit controls the enabling > > > of the > > > - * graphics AGP aperture for the AGP3.0 port. > > > + * 2. Enable AGP aperture in RX91<0>. This bit controls the > > > + * enabling of the graphics AGP aperture for the AGP3.0 port. > > this is okay. Except the missing spaces between the '*' and 'enabling'. > > > */ > > > pci_read_config_dword(agp_bridge->dev, VIA_AGP3_GARTCTRL, &temp); > > > - pci_write_config_dword(agp_bridge->dev, VIA_AGP3_GARTCTRL, temp | > > > (3<<7)); > > > + pci_write_config_dword(agp_bridge->dev, VIA_AGP3_GARTCTRL, > > > + temp | (3<<7)); > > this one is probably okay. I'd add some more spacing: + temp | (3 << 7)); The driver seems to suffer from a lack of spaces around << throughout. And most of those << should probably be defines somewhere ... -- Matthew Wilcox Intel Open Source Technology Centre "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." ^ permalink raw reply [flat|nested] 3+ messages in thread
[parent not found: <663ba5090810202137w750a1cc9m815489e92a2c6d94@mail.gmail.com>]
[parent not found: <663ba5090810220353g7da53f0mb57fe2129da5a8b2@mail.gmail.com>]
* Re: [PATCH] via-agp.c fixed compilation error and warnings for 2.6.26 [not found] ` <663ba5090810220353g7da53f0mb57fe2129da5a8b2@mail.gmail.com> @ 2008-10-22 11:48 ` Matthew Wilcox 0 siblings, 0 replies; 3+ messages in thread From: Matthew Wilcox @ 2008-10-22 11:48 UTC (permalink / raw) To: nagaraj s k Cc: Dave Airlie, trivial, greg, linux-kernel, kernel-janitors, nagaraj.krishnappa On Wed, Oct 22, 2008 at 04:23:37PM +0530, nagaraj s k wrote: > I tried making the changes as you rightly pointed out, please comment on > this patch and let me know if this still needs some work. I don't know what you're doing, but the original version of this file has tabs. You seem to have converted this to 4-space indents somehow. > @@ -29,14 +29,15 @@ static int via_fetch_size(void) > values = A_SIZE_8(agp_bridge->driver->aperture_sizes); > pci_read_config_byte(agp_bridge->dev, VIA_APSIZE, &temp); > for (i = 0; i < agp_bridge->driver->num_aperture_sizes; i++) { > - if (temp == values[i].size_value) { > - agp_bridge->previous_size = > - agp_bridge->current_size = (void *) (values + i); > + if (temp == values[i].size_value) > + continue; > + agp_bridge->previous_size = agp_bridge->current_size = > + values + i; > agp_bridge->aperture_size_idx = i; > return values[i].size; > } > - } > - printk(KERN_ERR PFX "Unknown aperture size from AGP bridge (0x%x)\n", > temp); > + dev_err(&agp_bridge->dev->dev, "Unknown AGP aperture size 0x%x\n", > + temp); > return 0; > } > > @@ -82,9 +83,9 @@ static void via_tlbflush(struct agp_memo > u32 temp; > > pci_read_config_dword(agp_bridge->dev, VIA_GARTCTRL, &temp); > - temp |= (1<<7); > + temp |= (1 << 7); > pci_write_config_dword(agp_bridge->dev, VIA_GARTCTRL, temp); > - temp &= ~(1<<7); > + temp &= ~(1 << 7); > pci_write_config_dword(agp_bridge->dev, VIA_GARTCTRL, temp); > } > > @@ -114,13 +115,15 @@ static int via_fetch_size_agp3(void) > temp &= 0xfff; > > for (i = 0; i < agp_bridge->driver->num_aperture_sizes; i++) { > - if (temp == values[i].size_value) { > - agp_bridge->previous_size = > - agp_bridge->current_size = (void *) (values + i); > + if (temp == values[i].size_value) > + continue; > + agp_bridge->previous_size = agp_bridge->current_size = > + values + i; > agp_bridge->aperture_size_idx = i; > return values[i].size; > } > - } > + dev_err(&agp_bridge->dev->dev, "Unknown AGP aperture size 0x%x\n", > + temp); > return 0; > } > > @@ -141,12 +144,13 @@ static int via_configure_agp3(void) > agp_bridge->gatt_bus_addr & 0xfffff000); > > /* 1. Enable GTLB in RX90<7>, all AGP aperture access needs to fetch > - * translation table first. > - * 2. Enable AGP aperture in RX91<0>. This bit controls the enabling of > the > - * graphics AGP aperture for the AGP3.0 port. > + * translation table first. > + * 2. Enable AGP aperture in RX91<0>. This bit controls the > + * enabling of the graphics AGP aperture for the AGP3.0 port. You've deleted the spaces at the beginning of the line here. Don't do that. > */ > pci_read_config_dword(agp_bridge->dev, VIA_AGP3_GARTCTRL, &temp); > - pci_write_config_dword(agp_bridge->dev, VIA_AGP3_GARTCTRL, temp | > (3<<7)); > + pci_write_config_dword(agp_bridge->dev, VIA_AGP3_GARTCTRL, > + temp | (3 << 7)); > return 0; > } > > @@ -156,7 +160,8 @@ static void via_cleanup_agp3(void) > struct aper_size_info_16 *previous_size; > > previous_size = A_SIZE_16(agp_bridge->previous_size); > - pci_write_config_byte(agp_bridge->dev, VIA_APSIZE, > previous_size->size_value); > + pci_write_config_byte(agp_bridge->dev, VIA_APSIZE, > + previous_size->size_value); > } > > > @@ -165,7 +170,8 @@ static void via_tlbflush_agp3(struct agp > u32 temp; > > pci_read_config_dword(agp_bridge->dev, VIA_AGP3_GARTCTRL, &temp); > - pci_write_config_dword(agp_bridge->dev, VIA_AGP3_GARTCTRL, temp & > ~(1<<7)); > + pci_write_config_dword(agp_bridge->dev, VIA_AGP3_GARTCTRL, > + temp & ~(1 << 7)); > pci_write_config_dword(agp_bridge->dev, VIA_AGP3_GARTCTRL, temp); > } > > @@ -421,13 +427,13 @@ static struct agp_device_ids via_agp_dev > * VIA's AGP3 chipsets do magick to put the AGP bridge compliant > * with the same standards version as the graphics card. > */ > -static void check_via_agp3 (struct agp_bridge_data *bridge) > +static void check_via_agp3(struct agp_bridge_data *bridge) > { > u8 reg; > > pci_read_config_byte(bridge->dev, VIA_AGPSEL, ®); > /* Check AGP 2.0 compatibility mode. */ > - if ((reg & (1<<1))==0) > + if ((reg & (1 << 1)) == 0) > bridge->driver = &via_agp3_driver; > } > > @@ -445,7 +451,8 @@ static int __devinit agp_via_probe(struc > return -ENODEV; > > j = ent - agp_via_pci_table; > - printk (KERN_INFO PFX "Detected VIA %s chipset\n", > devs[j].chipset_name); > + printk(KERN_INFO PFX "Detected VIA %s chipset\n", > + devs[j].chipset_name); > > bridge = agp_alloc_bridge(); > if (!bridge) > @@ -461,7 +468,8 @@ static int __devinit agp_via_probe(struc > if (pdev->device == PCI_DEVICE_ID_VIA_8367_0) { > /* Is there a KT400 subsystem ? */ > if (pdev->subsystem_device == PCI_DEVICE_ID_VIA_8377_0) { > - printk(KERN_INFO PFX "Found KT400 in disguise as a KT266.\n"); > + printk(KERN_INFO PFX "Found KT400 in disguise > + as a KT266.\n"); No. -- Matthew Wilcox Intel Open Source Technology Centre "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2008-10-22 11:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <663ba5090810050509g6ce3c25bh40686506b1d35b8f@mail.gmail.com>
[not found] ` <663ba5090810202049i16285109k34fde3ec1c9df319@mail.gmail.com>
2008-10-21 3:57 ` [PATCH] via-agp.c fixed compilation error and warnings for 2.6.26 Dave Airlie
2008-10-21 4:23 ` Matthew Wilcox
[not found] ` <663ba5090810202137w750a1cc9m815489e92a2c6d94@mail.gmail.com>
[not found] ` <663ba5090810220353g7da53f0mb57fe2129da5a8b2@mail.gmail.com>
2008-10-22 11:48 ` Matthew Wilcox
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox